Improve release workflow #390
Conversation
If it wasn't merged, fail with an error message.
…fore starting to build a release.
|
|
||
| from packaging.version import Version | ||
|
|
||
| FILE_APPEND_MODE = 'a' |
There was a problem hiding this comment.
Does this really need to be a constant?
There was a problem hiding this comment.
I copied this and the below from https://github.com/ansible-community/ansible-test-gh-action, it seems to be @webknjaz's preferred way of outputting things from a Python script in GHA. Maybe he can comment on it :)
There was a problem hiding this comment.
Yeah, I like to name the data. Otherwise, "what's is that weird 'a' string constant being passed to a function as a positional argument?" adds some cognitive load for people that are out of context. Some day, I'll move most of that to an actual Python file and this will reduce the boilerplate too..
|
|
||
| def set_output(name, value): | ||
| with OUTPUTS_FILE_PATH.open(FILE_APPEND_MODE) as outputs_file: | ||
| outputs_file.writelines(f'{name}={value}{os.linesep}') |
There was a problem hiding this comment.
Why not
| outputs_file.writelines(f'{name}={value}{os.linesep}') | |
| outputs_file.write(f'{name}={value}\n') |
? You shouldn't need writelines to write one line and \n does the same thing as os.linesep (in text mode, python converts \n to \r\n if that's os.linesep and we're not working with Windows to begin with).
There was a problem hiding this comment.
This is also copied from my code :)
I have helpers in other places where you can pass an iterable of strings and this is more genetic.
Co-authored-by: Maxwell G <maxwell@gtmx.me>
|
@felixfontein one thing I'd check is whether there's enough privileges in that job to query the PR data in that publish job. You may need a |
I don't think it really matters if the repo is public. |
| python3 -m pip install packaging | ||
| python3 -m pip install ansible-core | ||
| python3 -m pip install antsibull |
There was a problem hiding this comment.
TODO in a separate PR: merge these into one pip install command instead of three
There was a problem hiding this comment.
I was wondering whether the separate installs were intentional for some reason, so I added a third. But yeah, let's collapse them (in a separate PR).
Co-authored-by: Maxwell G <maxwell@gtmx.me>
I think so too; I guess we'll see when we try this out. I'd abuse the next 10.0.0 alpha for this kind of testing :) (I'll also try to approve the next step without merging the PR first, to see what happens. My guess is that restarting the failed second stage will work once the PR is merged.) |
@gotmax23 it does matter because setting permissions overrides the defaults and essentially revokes all other permissions that are not set explicitly. So the resulting |
|
https://github.com/ansible-community/ansible-build-data/actions/runs/8898520146/job/24436000110 Fixed in 1c10087. |
|
|
Next try: Fixed in acaf8bb. |
|
Ok, that fix was wrong, 9364168 should do better. |
|
Now it looks good: 🎉 |
The first commit stores the PR's URL and adds a step before uploading the release to PyPI which checks whether the PR has been merged. If it hasn't been merged, the
publishstage fails. (This isn't the best solution possible, but will prevent something like the accidental 9.5.0 release from today.)The second commit adds a Python step that parses the package version to validate it, and to extract the major version. This allows to simplify the workflow's input to a single version field.