ci: add cloud controller gql compat test#64092
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @michaellzc and the rest of your teammates on |
b5e9e30 to
394abf4
Compare
394abf4 to
4417b8d
Compare
ee7ea20 to
9d8fb27
Compare
070512a to
bc77a9c
Compare
f1ab798 to
2bb0e76
Compare
e2e5efa to
b9a974d
Compare
b9a974d to
2b21cc5
Compare
2c3d2aa to
4a0a4cd
Compare
There was a problem hiding this comment.
The rest of the PR looks good, nice work!
I feel like the whole comment managing part doesn't add a lot of value – it still sends the user to just browse the workflow logs in a separate repository.
What do you think about just printing (in workflow, not on PR) this message or similar:
## :x: Cloud Controller GraphQL Compatability Test Result
[sourcegraph/controller](https://github.com/sourcegraph/controller) uses the GraphQL API to perform automation. The compatibility test has failed and this pull request may have introduced breaking changes to the GraphQL schema.
Next steps:
- Review the GitHub Actions [workflow logs](https://github.com/sourcegraph/controller/actions/runs/${{ steps.workflow-dispatch.outputs.run-id }}) for more details.
- Reach out to the Cloud Ops team to resolve the issue before merging this pull request.
if the workflow fails? And print nothing if it succeeds.
I feel like this will have a similar quality of DX, and not introduce a dependency to a 3rd party workflow which also requires extra PR write permissions.
There was a problem hiding this comment.
I found people ignore status check (even more common is they’re not marked as required)
therefore, printing to PR has a better chance of catching the author’s attention. this is speaking from my past experience of building PR review style validation that requires human intervention.
how about we still post the comment in the PR, but only do so when there’s a failure?
There was a problem hiding this comment.
I was actually thinking of making this workflow required, but happy to wait until that is strictly necessary 👍
keynmol
left a comment
There was a problem hiding this comment.
Ready to approve, just wondering if we can get rid of additional complexity in the workflow with little to no impact to the DX.
4a0a4cd to
51bd604
Compare
Wonder what people's thoughts are on my existing implementation using actions/github-script https://sourcegraph.sourcegraph.com/github.com/sourcegraph/sourcegraph@nsc/execlog-diff/-/blob/.github/workflows/bazel-test-ownership-check.yml?L24%3A22-80%3A14 in terms of legibility? |
No strong opinion, I am happy to switch over. it's nice that we can just have one single step to represent all the logic |
88110fc to
921b1b0
Compare
@Strum355 updated |
keynmol
left a comment
There was a problem hiding this comment.
Nice!
One possible (and very optional) future refactor is to extract the logic into .github/workflows/cloud-gql-compat.js and run it like this: https://github.com/actions/github-script?tab=readme-ov-file#run-a-separate-file-with-an-async-function, so that it's easier to see/modify the script.
cd167f0 to
76edc5e
Compare
I gave it a try but couldn't figure out the right import path of the module. let's leave it for now |
76edc5e to
605e334
Compare
closes CLO-527
context
cloud uses a lot of the GraphQL API to pre-configure instances on customer behave, and it's very sensitive to breaking changes to the schema upstream.
currently, we have a cronjob github actions that periodically check the schema compatibility every day.
such approach works but we're always playing catch up and will have to result in extra work on the product team to re-work the PR. it is much better to catch this in CI time within the monorepo.
This PR added a new github actions to the monorepo that will run on any
*.graphqlchanges. Then it will remotely trigger the cronjob github action in sourcegraph/controller to run and poll the result. See test plan for demo.FAQ
Why not run the test in buildkite or directly in this repo using github actions?
How does authentication work?
We use a GitHub App with extremely limited permissions. It only permits the workflow to trigger/read the workflow without any access to the source code itself.
Also, GitHub App installation access token has a life span of 1h, much better than PAT.
Test plan
it triggered the job and it worked:
good: https://github.com/sourcegraph/sourcegraph/pull/64094
bad: https://github.com/sourcegraph/sourcegraph/pull/64095