DM-35150: Implement GitHub Checks for notebook execution #41
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jonathansick
force-pushed
the
tickets/DM-35150
branch
from
6c6c708
to
8d4e8d1
Compare
When set to a Git SHA, this associates the page with the Git commit of a GitHub Check Run. (This enables us to create pages during GitHub PR check runs)
Rather than having extensive logic for setting up a check run in the service layer, now class methods on the GitHubConfigsCheck are able to create and update the status of check runs. Overall the intent of this work is to make it easier to add check runs for notebooks in the next phase of work, and reduce the amount of complexity spread into the service layer.
This moves generic code for check domain models into a base class. The notebook execution will use this.
This is a first pass at running a github check for notebook execution. One thing that's a bit awkward is making PageService.add_page return a new PageExecutionInfo instance so the check run has access to the noteburst job corresponding to the page; it may be better to separate adding a page to the database from executing the notebook so that PageExecutionInfo doesn't need to have lots of nullable types.
This will allow us to add a client to this module and generalize it from only being models from noteburst responses created elsewhere.
This client, part of the noteburst domain, centralizes the API interactions with noteburst, which are now happening in both the page service and the github checks domain.
jonathansick
force-pushed
the
tickets/DM-35150
branch
from
6951278
to
71b83bb
Compare
The notebook execution check run is now able to set enable_retry=False to force a notebook execution to report issues quickly. In doing this, I've refactored the PageService methods to not force notebook execution, which makes is easier for different service use cases, like this one.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Building on #40, this PR implements a new GitHub Check Run that reports on whether a PR's notebook content can execute successfully. These notebooks are actually stored by Times Square, but tagged with the corresponding Git SHA. This will enable us to display a page with a preview of the notebooks in a GitHub pull request.