Add ability to rebuild a specific build #6995

ericholscher · 2020-04-29T19:19:02Z

This is most useful for PR builds,
but I've enabled it for all builds currently.
It might make sense to only enable it for PR builds,
or other builds where we care about rebuilding a specific commit.

It definitely isn't the final UX, but it works reasonably well for now:

Fixes #6524

saadmk11

Its Amazing to see this go through. This will surely help the users a lot. 💯

readthedocs/builds/views.py

readthedocs/core/utils/__init__.py

agjohnson

👍 on the feature, but could we either move the button or replace it with a link? This feels a bit unpolished if it's going to be live for all projects.

humitos

I'm not sure to understand the use case for this. What are the scenarios where you want to re-build the same commit? What would be different? I can see that the same commit could install different dependencies if they are not pinned for example, but that looks like an edge case to me.

I think the "rebuilding a build that failed because of RTD" --as the image shown, is kind of a hack and we should concentrate on having less of those.

ericholscher · 2020-05-18T22:59:31Z

I hit this pretty frequently when something weird happens with the PR builder. I think it's a useful enough thing to support, since other ways of supporting this are weird hacks.

This is most useful for PR builds, but I've enabled it for all builds currently. It might make sense to only enable it for PR builds, or other builds where we care about rebuilding a specific commit. Fixes #6524

Co-authored-by: Maksudul Haque <[email protected]>

agjohnson

Button placement looks good! I'm -1 on repurposing command output for state display, but I think skipping UX on this interaction is absolutely fine. We can revisit polishing this up with new templates.

agjohnson · 2020-05-19T15:19:22Z

readthedocs/core/utils/__init__.py

+        BuildCommandResult.objects.create(
+            build=build, command='Rebuilding build', exit_code=0,
+            start_time=datetime.utcnow(), end_time=datetime.utcnow(),
+        )


I'd probably remove this entirely, I don't think it's worth introducing UI confusion in the command output. The best place to solve this would be either popping up a confirmation box for the user on click, or simply revert the state back to show the UI as "loading". Either would communicate to the user that the build is restarting

I'm more worried about other folks looking at it and being confused about why it was running again after it had already built. I think we don't do a great job of showing history on builds -- we hit this issue with failures & retries as well, so we need some kind of pattern for it.

I've updated this to use the new build status, which is a better solution 👍

…rebuild-builds

ericholscher · 2020-06-23T17:50:55Z

This should be ready for a final review with the new status change 👍

stale · 2020-08-08T02:46:01Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

…rebuild-builds

stale · 2020-09-26T14:49:16Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

humitos · 2021-06-02T09:50:09Z

I took a look at this PR today trying to recover the feature and finish it.

@ericholscher is there a strong reason why we are re-triggering the same build object instead of just trigger a new build with a specific commit? IMO, reading the code just makes it more complex passing the build object around different methods, adding a new build status, etc.

I'm thinking that we can just call trigger_build(commit=) with the commit received from the dashboard (via ?commit= or via ?build_pk= if the build is associated with a particular commit). With this approach, it would create a new Build object and we would keep all the build outputs, giving users more context about why the first time it fails and maybe the subsequent re-triggered ones passed.

I'll try opening a PR with this idea.

humitos · 2021-06-02T10:38:51Z

I opened a PR at #8227 to show my idea but I'm still not convinced this is a good feature and I think it will create more problems than benefits.

While it may be a good feature to "re-build the latest failed build from a PR", it will for sure create problems for all the other use cases that involve an old commit:

re-build an old commit from this external version
re-build an old commit from latest
re-build an old commit from stable
re-build an old commit from any other version

In all these cases, the user will be building and publishing an old version of their documentation, and they may not even notice that they just break their docs. They will end up with that version of their (now out-of-date) docs pointing to an old commit.

Basically, there is going to be a discrepancy between what you have in your repository and what you have published, breaking one of the main benefits of RTD. Read the Docs' case is different from a regular CI because it does not only do checks but also generates and publishes assets.

Considering the best use case for this feature the one I mentioned in the first paragraph, there are currently at least some workarounds that are not ideal but way safer from a user's perspective:

add a new commit to the PR, or
close and re-open the PR

If any, it seems we should only allow to re-build on that particular case. Still, I don't think it adds a lot of value.

astrojuanlu · 2021-06-02T11:55:16Z

I agree with the concerns raised by @humitos. I still think this is a good feature though, provided that we narrow down the scope.

Reopening and closing the PR also triggers all other CI checks users have. This might be undesirable.
The most likely reason to do this is that some temporary network failure ruined the build. Therefore, I don't think users will want to build ancient commits. And if they want, they already have other methods.

Therefore, I think we could limit the feature as follows:

Only rebuilds of PR builds are allowed. No danger in rebuilding an old published version, that should be done with the existing UI.
Does not allow rebuilding ancient commits. Therefore, we can push a reasonable timeout, say 24 hours.

What do you think? Pinging @pllim , who probably has interest in this (I don't remember if we are discussing the feature in some other issue).

stsewd · 2021-06-02T14:15:15Z

I think the idea was to allow to re-build a PR or version from the latest commit.

pllim · 2021-06-02T15:29:22Z

24 hours might not work for some. A commit can still be a latest commit but weeks old before a reviewer can get to it and realized a rebuild is needed, though I guess in that case, reviewer can open/close as usual and cancel the other CI (not pretty but it is a workaround).

@astrojuanlu , I do vaguely recall talking about this though what I wanted more is the ability to skip building RTD entirely via [skip ci], [ci skip], [skip rtd], and [rtd skip].

astrojuanlu · 2021-06-02T15:30:41Z

Ah yes, that's #7660 :)

ericholscher · 2021-06-02T23:04:02Z

Agreed with @astrojuanlu -- I think we probably only want to allow it for the last build on a PR branch. It should be simple enough to figure this out (pk == Build.objects.filter(version=pr_version).first()). I don't feel strongly about reusing a build object or creating a new one. I initially reused it so that the URL would be the same. I don't think either is obviously better in terms of UX, so whatever is simpler is probably best.

Allowing to re-build all the versions and all the past build could end up on publishing outdated docs for a stable/latest version. So, we are restricting this only to External Versions and only to its latest build object. See discussion in #6995 (comment)

pllim · 2021-06-09T11:59:28Z

I cannot wait to put this into good use. Thanks! 😸

dimpase · 2021-12-01T13:25:45Z

I'm not sure to understand the use case for this. What are the scenarios where you want to re-build the same commit?

In our case we have to rely on our package (https://pypi.org/project/primecountpy/) supplied via a wheel (it depends on an external C++ library, and the rtd builder
cannot handle building the package). Now, our wheels are built on GitHub, using https://github.com/pypa/cibuildwheel, which is triggered by push/PR.
If the wheel is broken, it need not lead to an error in rtd builder; it often passes, but the docs are broken.
So we cannot rely on the rebuild button only available for errored out builds.

Ideally, we would like to be able to install wheels on PyPI and only then trigger the rtd rebuild, but we'd be OK with rebuild
button being available for passed builds (after we install the wheels to PyPI, we can just press it...)

astrojuanlu · 2022-01-11T12:16:56Z

@dimpase sorry it took so long to respond. Would an incoming webhook suit your use case? You can trigger builds externally https://docs.readthedocs.io/en/stable/integrations.html#using-the-generic-api-integration

ericholscher added the Needed: tests Tests are required label Apr 29, 2020

ericholscher requested a review from a team April 29, 2020 19:19

saadmk11 reviewed Apr 30, 2020

View reviewed changes

readthedocs/builds/views.py Outdated Show resolved Hide resolved

readthedocs/core/utils/__init__.py Outdated Show resolved Hide resolved

agjohnson requested changes May 8, 2020

View reviewed changes

humitos reviewed May 11, 2020

View reviewed changes

ericholscher and others added 5 commits May 18, 2020 16:24

Add ability to rebuild a specific build

13a9e7c

This is most useful for PR builds, but I've enabled it for all builds currently. It might make sense to only enable it for PR builds, or other builds where we care about rebuilding a specific commit. Fixes #6524

Update readthedocs/core/utils/__init__.py

d0e715c

Co-authored-by: Maksudul Haque <[email protected]>

Update readthedocs/builds/views.py

1f74b9a

Co-authored-by: Maksudul Haque <[email protected]>

Fix rebuild link display

3a6d1e6

Add test for building by commit

a2c731a

ericholscher force-pushed the rebuild-builds branch from 16fd3e7 to a2c731a Compare May 18, 2020 23:24

Add a BuildCommandResult for retriggering ot make the UI nicer

423c3d8

agjohnson requested changes May 19, 2020

View reviewed changes

ericholscher removed the Needed: tests Tests are required label May 19, 2020

ericholscher added 2 commits June 23, 2020 10:48

Merge branch 'master' of github.com:readthedocs/readthedocs.org into …

5d51f58

…rebuild-builds

Use new build status to convey retriggered state

0a0221e

stale bot added the Status: stale Issue will be considered inactive soon label Aug 8, 2020

Merge branch 'master' of github.com:readthedocs/readthedocs.org into …

57ee3dc

…rebuild-builds

ericholscher requested a review from a team August 8, 2020 15:06

stale bot removed the Status: stale Issue will be considered inactive soon label Aug 8, 2020

ericholscher requested a review from agjohnson August 8, 2020 15:06

stale bot added the Status: stale Issue will be considered inactive soon label Sep 26, 2020

saadmk11 removed the Status: stale Issue will be considered inactive soon label Sep 27, 2020

ericholscher added the Accepted Accepted issue on our roadmap label Sep 28, 2020

humitos mentioned this pull request Jun 2, 2021

Add ability to rebuild a specific build #8227

Merged

humitos merged commit 53cc567 into master Jun 9, 2021

humitos deleted the rebuild-builds branch June 9, 2021 07:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ability to rebuild a specific build #6995

Add ability to rebuild a specific build #6995

ericholscher commented Apr 29, 2020 •

edited

Loading

saadmk11 left a comment

agjohnson left a comment •

edited

Loading

humitos left a comment

ericholscher commented May 18, 2020

agjohnson left a comment

agjohnson May 19, 2020

ericholscher May 19, 2020

ericholscher Jun 23, 2020

ericholscher commented Jun 23, 2020

stale bot commented Aug 8, 2020

stale bot commented Sep 26, 2020

humitos commented Jun 2, 2021

humitos commented Jun 2, 2021

astrojuanlu commented Jun 2, 2021

stsewd commented Jun 2, 2021

pllim commented Jun 2, 2021

astrojuanlu commented Jun 2, 2021

ericholscher commented Jun 2, 2021

pllim commented Jun 9, 2021

dimpase commented Dec 1, 2021 •

edited

Loading

astrojuanlu commented Jan 11, 2022

Add ability to rebuild a specific build #6995

Add ability to rebuild a specific build #6995

Conversation

ericholscher commented Apr 29, 2020 • edited Loading

saadmk11 left a comment

Choose a reason for hiding this comment

agjohnson left a comment • edited Loading

Choose a reason for hiding this comment

humitos left a comment

Choose a reason for hiding this comment

ericholscher commented May 18, 2020

agjohnson left a comment

Choose a reason for hiding this comment

agjohnson May 19, 2020

Choose a reason for hiding this comment

ericholscher May 19, 2020

Choose a reason for hiding this comment

ericholscher Jun 23, 2020

Choose a reason for hiding this comment

ericholscher commented Jun 23, 2020

stale bot commented Aug 8, 2020

stale bot commented Sep 26, 2020

humitos commented Jun 2, 2021

humitos commented Jun 2, 2021

astrojuanlu commented Jun 2, 2021

stsewd commented Jun 2, 2021

pllim commented Jun 2, 2021

astrojuanlu commented Jun 2, 2021

ericholscher commented Jun 2, 2021

pllim commented Jun 9, 2021

dimpase commented Dec 1, 2021 • edited Loading

astrojuanlu commented Jan 11, 2022

ericholscher commented Apr 29, 2020 •

edited

Loading

agjohnson left a comment •

edited

Loading

dimpase commented Dec 1, 2021 •

edited

Loading