Build: report "Build canceled" when sending VCS build status' description for a particular commit

readthedocs / readthedocs.org

The source code that powers readthedocs.org

https://readthedocs.org/

MIT License

7.92k stars 3.58k forks source link

Build: report "Build canceled" when sending VCS build status' description for a particular commit #8998

Open pllim opened 2 years ago

pllim commented 2 years ago

Example: astropy/astropy#12928

PR status: docs/readthedocs.org:astropy Pending — Read the Docs build is in progress!

But when you click on "Details", it goes to https://readthedocs.org/projects/astropy/builds/16281860/ that says Error - Build cancelled by user.

Expectation: PR status says "build has been canceled by user" or something to that effect with a ❌ .

humitos commented 2 years ago

Hi @pllim! I'm not able to reproduce this issue. I tried with our testing project and it worked as you expect https://github.com/readthedocs/test-builds/pull/2082

Does the Github status report work properly for your project usually? I saw a message in the logs saying that there were permissions problems and the status wasn't able to be sent.

pllim commented 2 years ago

I think usually it works. I'll keep an eye out and report back though .

stsewd commented 2 years ago

The message says the build failed

docs/readthedocs.org:test-builds — Read the Docs build failed!

I think it is better if the message says the build was cancelled instead.

pllim commented 2 years ago

Yes, now it reports back as failed. Maybe what I originally reported was transient.

I think it is better if the message says the build was cancelled instead.

I completely agree. That would be more consistent with other CI providers. Thanks!

humitos commented 2 years ago

Related to #9040

humitos commented 2 years ago

To make this possible, the code needs some refactoring. Currently, we are only sending the state (failed, pending, success) and we use a constant SELECT_BUILD_STATUS to find the description to be sent. However, we would need either:

pass the reason to the function that sends the message (https://github.com/readthedocs/readthedocs.org/blob/ca4b3c79556b32341b51fd36788b4cb314993187/readthedocs/oauth/services/github.py#L438-L439)
pass the description itself when the function is called

To do this, we need to modify the util function, the task, and both service methods at least.

zmbc commented 10 months ago

Beyond just the message, it is surprising to me that cancelling the build returns a failure status. I know that GitHub Actions for instance can return a "cancelled" status, which doesn't have a red X that makes things look like they failed. Is that a feature GitHub allows third-party tools to use? Or, could it make more sense to just leave the status as pending?