Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update ci.yml to send correct payload information to benchmark repo #3

Closed
mikemimik opened this issue Dec 4, 2019 · 3 comments
Closed
Assignees

Comments

@mikemimik
Copy link
Contributor

mikemimik commented Dec 4, 2019

Currently we have to run the following line to get the benchmark suite to work. The https://github.com/npm/cli repo has to send a request to this repository which is private, which means auth is needed.

https://github.com/npm/cli/blob/latest/.github/workflows/benchmark.yml#L38

Changing this repository to public should negate the need for auth. Allowing this request to just be sent.

Testing

  • should be able to validate this with just some curl requests
@mikemimik mikemimik self-assigned this Dec 4, 2019
@mikemimik
Copy link
Contributor Author

Made the repo public. Still need to validate that the npm/cli project doesn't need to send the authorization headers

This was referenced Dec 6, 2019
@mikemimik
Copy link
Contributor Author

mikemimik commented Dec 6, 2019

Situation

With regards to fixing the npm/benchmarks repo so that all pull-requests (specifically those created by those in the community) will have the suite run against them.

The dispatch endpoint is an authenticated endpoint, so you have to send a request that has a Personal Access Token (PAT) attached to it. The PAT needs to be from an account that has access to the repo, even though the repo is public. I suppose this makes sense otherwise anyone could trigger the events that you're repository is relying on.

This does mean that only pull-requests that are opened in the npm/cli repo will trigger the benchmarks. If we're okay this this, it just means that the release-x.y.z branch needs to have a pull-request created for it, so that the benchmark suite will run. If we do care that contributed pull-requests have the suite run, then we'll need to come up with a solution.

Suggestion

This should be broken out into a separate ticket/story, for now. Making the repository public was the hypothesis to supporting community pull-requests. While not the case, this repository can (and should) still remain public.

Possible Solution

A GitHub App that we own running on a server somewhere (heroku?), which does this dispatching for us. The npm/cli repo just needs to have a webhook setup that upon a pull-request to send that POST to the GitHub App. The app then does what the GitHub Action used to do, which is send a dispatch to the https://api.github.com/repos/npm/benchmarks/dispatch endpoint.

@darcyclarke darcyclarke added this to the OSS - Sprint 1 milestone Dec 17, 2019
@mikemimik mikemimik changed the title Make benchmark repo public Update ci.yml to send correct payload information to benchmark repo Jan 7, 2020
@mikemimik
Copy link
Contributor Author

fixed with npm/cli#689

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
@darcyclarke @mikemimik and others