GitHub as event stream proof-of-concept #44
Closed
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
Start adding some tests to Homu by testing the issue comment command-parsing logic. Take `parse_commands` and break it apart into two phases * Parsing phase * Execution phase The parsing phase returns a list of commands with action names (ideally, this would be a Rust enum, but to simulate that, we use action names on a single class) that are returned regardless of the current state. So for example, `@bors retry` will return a `retry` command regardless of the current state of `realtime`. The execution phase then inspects the list of commands and decides what to do with them. So for example, the `retry` command will be skipped if `realtime == False`. This has the positive result of having a parsing phase that has no side-effects, which makes it much easier to test. This can lead to higher confidence that the code works as expected without the high cost of testing in production and possibly disrupting the build flow. This has the negative result of adding a lot of lines of code to achieve command parsing, which we already do successfully without the tests.
Use `python setup.py test` to run the tests, and a few minor changes required to support that.
Fix merge conflict in homu/main.py
We currently have two sources-of-truth when syncing on startup: the database and issue comments. This proof of concept tries to bring us down to a single source-of-truth by effectively elimating the local database (at least for pull request state; other aspects like `treeclosed` are not addressed). In order to do this, we use GitHub's "timeline" ([v3](https://developer.github.com/v3/issues/timeline/), [v4](https://developer.github.com/v4/object/pullrequest/#timelineitems)) feature to get a fuller view of what has happened to a pull request so we can reify state in a more complete manner than using issue comments alone. For example, the timeline API interleaves issue comments with things like `AssignedEvent`, `HeadRefForcePushedEvent`, and `RenamedTitleEvent` in the order they happened so we can reliably build the full state from this historical record. The general concept is that in both `synchronizing` and `realtime` mode, we have the pull request object respond to the event to update the state. Side effects like comments and label updates are returned, and the code can use or drop those depending on which mode Homu in. PR state --------\ | +---------------+ \---> | | -----> Modified PR state | process_event | /---> | | --\ | +---------------+ +--> Realtime comments GitHub Event ----/ +--> Label updates \--> Database update The good thing is that we can model these timeline items and GitHub's event webhooks (which we use when processing in `realtime` mode) in much the same way, so that we can unify them in a way that makes it possible to effectively test the effect of events on a pull request. This particular pull request does not attempt to integrate these changes into the codebase yet, because I wanted to get feedback that this is indeed a viable path to head down. Instead, this pull request (which should not be merged) provides a new executable that demonstrates this ability on existing pull requests in a read-only mode. The output is a series of relevant timeline items, and the state of the pull request as it changes. pip install -e homu homu-sync-test --repo rust-lang/rust 61419
|
@kennytm @pietroalbini @Manishearth (not sure who else) I'd love to get some 👀 on this, and your thoughts on this approach. Might be some significant cutting to get there so I wanted to get feedback first, but I think it could have some real tangible benefits in the future, too. |
|
+1 for reducing the SQLite dependency |
|
This approach seems awesome! |
7 tasks
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.
We currently have two sources-of-truth when syncing on startup: the
database and issue comments.
This proof of concept tries to bring us down to a single source-of-truth
by effectively elimating the local database (at least for pull request
state; other aspects like
treeclosedare not addressed).In order to do this, we use GitHub's "timeline"
(v3,
v4)
feature to get a fuller view of what has happened to a pull request so
we can reify state in a more complete manner than using issue comments
alone. For example, the timeline API interleaves issue comments with
things like
AssignedEvent,HeadRefForcePushedEvent, andRenamedTitleEventin the order they happened so we can reliably buildthe full state from this historical record.
The general concept is that in both
synchronizingandrealtimemode,we have the pull request object respond to the event to update the
state. Side effects like comments and label updates are returned, and
the code can use or drop those depending on which mode Homu in.
The good thing is that we can model these timeline items and GitHub's
event webhooks (which we use when processing in
realtimemode) in muchthe same way, so that we can unify them in a way that makes it possible
to effectively test the effect of events on a pull request.
This particular pull request does not attempt to integrate these changes
into the codebase yet, because I wanted to get feedback that this is
indeed a viable path to head down.
Instead, this pull request (which should not be merged) provides a new
executable that demonstrates this ability on existing pull requests in a
read-only mode. The output is a series of relevant timeline items, and
the state of the pull request as it changes.