Implement GitHub webhooks #69
Conversation
danlivings-dxw
force-pushed
the
feature/use-sqlite-db
branch
from
95df3c1
to
ea205a0
Compare
danlivings-dxw
force-pushed
the
feature/use-github-webhooks
branch
3 times, most recently
from
61f8d54
to
6f0fd81
Compare
danlivings-dxw
force-pushed
the
feature/use-github-webhooks
branch
from
6f0fd81
to
0ed3848
Compare
There was a problem hiding this comment.
It's been several years since I was regularly in js world - is it standard practice to not keep tests in a dedicated dir?
There was a problem hiding this comment.
I've mostly been following the lead set by Rich on this when he was still at dxw - I'm not clued up enough on JS best practices to know whether or not having separate source and test folders is preferred or not.
There was a problem hiding this comment.
Unit tests yes (so this is fine), integration tests no
This will allow webhooks to call this function to ensure that new dependencies have lifetime information if it is available.
The logic used to fetch and store information about repositories and dependencies will need to be reused to allow webhooks to make changes to the database. Using the logic in the application rather than in one-off seeding scripts makes it more important that it is tested to avoid unexpected changes to behaviour. This moves the actual seed scripts to wrapper scripts in the `seed` folder, which call the methods in the original files that have been refactored to be more testable.
To update dependency or repository information in an efficient way, Towtruck should listen to GitHub events rather than polling for updates. Towtruck needs to listen to the following events: - `pull_request.opened` and `pull_request.closed` to update the list of open pull requests - `issues.opened` and `issues.closed` to update the list of open issues - `push` and `issues.edited` to update the list of dependencies - `dependabot_alert` to update the list of vulnerabilities - `repository` to update basic information about repositories
danlivings-dxw
force-pushed
the
feature/use-github-webhooks
branch
from
0ed3848
to
25aeb2a
Compare
|
Thanks for tackling this feature, Dan, there's a lot of complexity at work and it looks like your refactoring and test improvements will improve readability and maintainability of the codebase. I've found it a little tricky to review due to the number of changes here (+1,484 −67!). As someone who likes to review a PR commit by commit, I’m finding it a little difficult to review the second refactoring commit here with the number of changes, and I’m a little concerned I’ll miss something. Suggestion: Would it be possible to split the big refactor into its own PR made up of smaller commits? Here’s how I’d structure it:
I’d find it much easier to digest the changes in these smaller chunks, and could hopefully give you a more thorough review. This PR could then consist of a few smaller commits, one for each of the Github events. |
This PR allows Towtruck to listen for several types of events on the
POST /api/github/webhooksendpoint. This can be used to incrementally update the database and spreading out the requests it makes to the GitHub API, rather than issuing a large number of requests in a short window of time as it does when seeding.The events it listens for are:
pull_request.openedandpull_request.closedto update the list of open pull requestsissues.openedandissues.closedto update the list of open issuespushandissues.editedto update the list of dependenciesdependabot_alertto update the list of vulnerabilitiesrepositoryto update basic information about repositoriesNB: this pull request depends on #63, which should be merged first.