Skip to content

Add workflow to dispatch the update of the OpenAPI spec #11709

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

Merged
merged 2 commits into from
Oct 21, 2024

Conversation

giograno
Copy link
Member

@giograno giograno commented Oct 17, 2024

Motivation

We are building a separate repo to store LocalStack's OpenAPI specs.
This workflow is triggered when the spec changes and it dispatches a workflow on the destination repo that generates a new latest spec.

See https://github.com/localstack/localstack-ext/pull/3554 for more context.

@giograno giograno added the semver: patch Non-breaking changes which can be included in patch releases label Oct 17, 2024
@giograno giograno changed the title Add workflow to dispatch the update of the OpenAPI spec. Add workflow to dispatch the update of the OpenAPI spec Oct 17, 2024
Copy link

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   1h 42m 12s ⏱️ - 1m 3s
3 508 tests ±0  3 095 ✅ ±0  413 💤 ±0  0 ❌ ±0 
3 510 runs  ±0  3 095 ✅ ±0  415 💤 ±0  0 ❌ ±0 

Results for commit 5dc5cc9. ± Comparison against base commit 56f6a78.

@giograno giograno marked this pull request as ready for review October 18, 2024 11:50
Copy link
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The workflow is looking good, but I would discuss the usage patterns for the client-payload config before merging this PR. I think this could be problematic, but could maybe also be useful?

token: ${{ secrets.PRO_ACCESS_TOKEN }}
repository: localstack/openapi
event-type: openapi-update
client-payload: '{"ref": "${{ github.ref }}", "repo": "${{ github.repository }}"}'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: Is this really about handing over the reg and repo here? This would mean that if you trigger this manually from another branch, the specs would be taken from another branch, updating the specs in the repo with a non-merged version.
Is this what we want? What would this be used for?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair observation. I added the ref to the branch and the workflow_dispatch to be able to test the workflow from some spec changes sitting in a branch.
In my view, this workflow should be dispatched only when a spec change is pushed on master. Mostly for testing purposes, it could be handy to have the chance to trigger it from another branch, but I can't come up with concrete examples right now. The spec won't be automatically updated anyway: a PR would still have to be approved and merged.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right that makes sense! The additional barrier with the PR is great and should definitely be enough!
Would you mind adding a comment to the client-payload? I feel like this isn't obvious and might make sense to add a comment here?

Copy link
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for answering the question! I think it would be great to add a small comment on the line discussed, but that's just a nitpick / not blocking a merge! 🚀

@giograno giograno merged commit 4ad57db into master Oct 21, 2024
16 of 24 checks passed
@giograno giograno deleted the dispatch-spec-update branch October 21, 2024 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants