Skip to content

Add script to release the SDK along the OpenAPI spec #16

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 11 commits into from
May 6, 2025
Merged

Conversation

giograno
Copy link
Member

@giograno giograno commented May 3, 2025

So far, we have manually released the Python SDK, generating the code from the latest OpenAPI spec.
This PR introduces a workflow to automate this process.

Given a LS version in input, we:

  • fetch the corresponding OpenAPI spec from the release artifacts in https://github.com/localstack/openapi;
  • update the generated code (i.e., generating code from the fetched spec);
  • pull the right LocalStack version and run the SDK test suite;
  • create and tag a release commit;
  • build and push to PyPi
  • create a draft release of the SDK;
  • create and push a dev version.

The workflow can be manually triggered on an arbitrary version of LS.
To properly integrate this into the release of LS, we can introduce a workflow triggered when a new OpenAPI spec is released that triggers this via repository_dispatch.

@giograno giograno requested a review from alexrashed May 3, 2025 10:02
@giograno giograno self-assigned this May 3, 2025
@giograno giograno marked this pull request as ready for review May 5, 2025 08:11
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.

Awesome! Really nice push towards further streamlining the release pipeline on LocalStack releases! 💯
I only added a bunch of nitpicks, nothing critical, so from my perspective this can be merged as is, or we can tackle these nitpicks right away ;)

- name: "Generate code from spec"
run: |
make clean-generated
./bin/generate.sh ${SPEC_URL}
Copy link
Member

Choose a reason for hiding this comment

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

nitpick (non-blocking): ‏Maybe it would be good to have all the "spec url fiddling" in the script itself? I think it would be nicer / easier to use if the script just takes an optional version argument and in the script it either uses the latest spec or constructs the URL based on the version. What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the suggestion 👍 definitely cleaner, and we could also drop 2 steps from the workflow. Done in 892bf4d


- name: "Commit changed code"
run: |
if git status --porcelain packages/ | grep -q '^'; then
Copy link
Member

Choose a reason for hiding this comment

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

nitpick (non-blocking): ‏That's an interesting condition, could you maybe add a comment here to explain what's going on in this line?

Comment on lines +79 to +81
- name: "Install LocalStack"
run: |
pip install localstack==${{ env.release }}
Copy link
Member

Choose a reason for hiding this comment

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

nitpick (non-blocking): The installation of the right version of localstack is just defined here in the pipeline. But it seems there is also a requirements-spec.txt (which defines localstack as a dependency, but might not be used at all anymore?). I think it would be nice to:

  • Clean up / remove the requirements file
  • Either extend the readme explaining that one needs to start the right version of LocalStack manually, or integrate an automatic startup of the right version of LocalStack into the pytest conftest / setup.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch. The requirement file is just a leftover from previous iterations.

giograno and others added 4 commits May 6, 2025 10:43
Co-authored-by: Alexander Rashed <2796604+alexrashed@users.noreply.github.com>
Co-authored-by: Alexander Rashed <2796604+alexrashed@users.noreply.github.com>
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 this pull request may close these issues.

2 participants