Send Build Status Report Using GitHub Status API #5865
Conversation
saadmk11
added
the
PR: work in progress
saadmk11
changed the title
readthedocs/core/utils/__init__.py
Outdated
| @@ -125,6 +128,10 @@ def prepare_build( | |||
| options['soft_time_limit'] = time_limit | |||
| options['time_limit'] = int(time_limit * 1.2) | |||
|
|
|||
| if build: | |||
| # Send pending Build Status to Git Status API | |||
| send_build_status.delay(build.id, BUILD_STATUS_PENDING) | |||
There was a problem hiding this comment.
not sure if calling this from here is the best place. I don't have a suggestion yet, but I'll try to dig the code. Also, we have signals to report the build status, maybe we can use those
There was a problem hiding this comment.
@stsewd I couldn't not find a better place for this . we should send the status report as soon as possible. please suggest me a better place when you get time :)
saadmk11
force-pushed
the
github-status-api
branch
from
09ca1ad to
9b10ea6
Compare
saadmk11
removed
the
PR: work in progress
| self.send_build_status( | ||
| self.build['id'], BUILD_STATUS_FAILURE | ||
| ) | ||
| elif self.build_env.successful: |
There was a problem hiding this comment.
What is the other state here? Should this just be an else? If not, we should do an else with a log.warning.
There was a problem hiding this comment.
@ericholscher there are no other states (BuildEnvironment) , it will succeed or fail (in the Build Environment) at this point of the build. we could add an warning if we want. but I don't think we need to :)
| # TODO: Send build status for other providers. | ||
|
|
||
|
|
||
| def send_external_build_status(build_pk, state): |
There was a problem hiding this comment.
Shouldn't this function be in utils instead of tasks?
There was a problem hiding this comment.
This will just create a circular import error, as the send_build_status task is already in the task.py file
| tasks.send_build_status(external_build, BUILD_STATUS_SUCCESS) | ||
|
|
||
| send_build_status.assert_not_called() | ||
| mock_logger.info.assert_called_with( |
There was a problem hiding this comment.
I don't think mocking the logger is needed for this test
| self.assertFalse(success) | ||
| mock_logger.exception.assert_called_with( | ||
| 'GitHub commit status creation failed for project: %s', | ||
| self.project, |
There was a problem hiding this comment.
Looks like there is an extra level of indentation here
|
🎉 |
Sending build status
pendingandsuncessorfailedusing GitHub Status API.Will extend this for other services in the future (e.g: gitlab, bitbucket)
TODO: