Fix github build status reporting bug #5985
Conversation
|
@ericholscher Build object does not have the commit sha when the build is triggered. Its added while building. so to send the triggered/pending status we need to send the version identifier |
|
We can change the code to send the commit when triggering the build though, right? We have the data at least for external versions. |
|
Yes we can do that. do you think that is a better approach then this? |
|
Yes. This can still hit bugs when a commit will switch between triggering and building. We should be explicitly passing a commit along if we have it. |
There was a problem hiding this comment.
I think this is a better approach. We've still not confirming that we're actually building the commit. We're not passing it into the VCS code to actually make sure we're checking it out and building, which is what we really need to be doing.
readthedocs.org/readthedocs/projects/tasks.py
Line 688 in af20054
We're also still checking out the identifier from the Version, and not the actual commit we're passing in:
readthedocs.org/readthedocs/projects/tasks.py
Line 156 in bb8b08f
| @@ -102,6 +104,7 @@ def prepare_build( | |||
| kwargs = { | |||
| 'record': record, | |||
| 'force': force, | |||
| 'commit': commit, | |||
There was a problem hiding this comment.
I think we should also be passing commit into the Build object here when created here.
readthedocs/projects/tasks.py
Outdated
| version_repo.update() | ||
| self.sync_versions(version_repo) | ||
| identifier = self.commit or self.version.identifier | ||
| version_repo.checkout(self.version.identifier) |
There was a problem hiding this comment.
I think we should pass this here. identifier from the line above. might have changes while resolving conflict :)
There was a problem hiding this comment.
Ah yea, that was a failure in the conflict resolution on my side.
This will fix the bug that was sending status reports for wrong commits.This happens when multiple commits are added to a pull request in a short amount of time and versions get updated with new identifiers every time so, that's why when sending the finish status report (
success/failure) it might send it for a different commit sha and give some inconsistent result.