-
Notifications
You must be signed in to change notification settings - Fork 8
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
chore: fix up release pipelines #17
Conversation
@jvanstraten , it seems like you've taken a different approach to what we used in the core repository. I think it would be better to use a consistent approach. Want to share the tokens with me via Slack so I can register them as necessary? |
I'm open to suggestions for consistency; I'd also like to keep things as similar as possible. The actual release pipelines are much more complicated than Substrait's though, requiring several platforms and (for Python) a docker container; I can't just do it from within a .sh file running in a single ubuntu-latest runner. So, I figured the semantic release script would just up the version, handle the release log, and create the release log part of the github release. It doing that should then trigger the pipelines that do the actual building and publishing. As for the tokens: I can pass you the tokens for my accounts, but wouldn't it be better to have accounts specifically for Substrait? ETA: To be clear, I just haven't gotten around to integrating semantic release and conventional commits yet. This PR isn't supposed to be mutually exclusive, rather I see it and #16 as dependencies. I'm just trying to keep the changes as chunked up as possible. |
Just sent the API tokens via Slack. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reviewed the Python code, but I think we should try to find a way to avoid it entirely.
working-directory: rs | ||
env: | ||
CARGO_REGISTRY_TOKEN: ${{ secrets.CARGO_REGISTRY_TOKEN }} | ||
run: cargo publish --allow-dirty |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can't these crates be published independently?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because
- one depends on the other;
- especially with automated releases it seems like a good idea to always release both to not complicate versioning, even if the derive crate is really unlikely to change;
- cargo won't let you publish a crate if a dependency is not yet in the registry;
- cargo publish exits with success before the published crate is synchronized into the crate registry (this can take several minutes depending on queue length).
The latter is an open issue. Most people use shell scripts, I strongly prefer Python personally (if only because it's already a dependency, and bash is not [ETA: though this is running on CI. d'oh, nevermind]). Once cargo resolves this, the blocking Python script can be burned at the stake.
Tested and fixed the release pipelines on my fork. A release is triggered by pushing a
vX.Y.Z
tag, which I assume could at some point be done by semantic release. The first version should be > 0.0.4 because it took a few tries to get everything to work, but the first real version should probably be 0.1.0 anyway.The release workflows depend on #16 for the version identifier file, so it should probably be merged first.
Repo admin stuff needed (@jacques-n):
CARGO_REGISTRY_TOKEN
repository secret set to a token for some crates.io account for Substrait.PYPI_API_TOKEN
repository secret set to a token for some PyPI account for Substrait.