Skip to content
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

Modify providers release process for transition to trusted publishing #44941

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Dec 15, 2024


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

Copy link
Member

@gopidesupavan gopidesupavan left a comment

Choose a reason for hiding this comment

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

Thanks Jarek for this changes. Now it's lot more easy in the final package identification. I will update the necessary changes in airflow-publish repo to align with these changes.

@eladkal
Copy link
Contributor

eladkal commented Dec 15, 2024

I'll review tommorow

dev/README_RELEASE_PROVIDER_PACKAGES.md Outdated Show resolved Hide resolved
Comment on lines -1236 to -1265
## Publish the packages to PyPI

By that time the packages should be in your dist folder.

```shell script
cd ${AIRFLOW_REPO_ROOT}
git checkout <ONE_OF_THE_RC_TAGS_FOR_ONE_OF_THE_RELEASED_PROVIDERS>
```

example `git checkout providers-amazon/7.0.0rc2`

Note you probably will see message `You are in 'detached HEAD' state.`
This is expected, the RC tag is most likely behind the main branch.

* Verify the artifacts that would be uploaded:

```shell script
twine check ${AIRFLOW_REPO_ROOT}/dist/*.whl ${AIRFLOW_REPO_ROOT}/dist/*.tar.gz
```

* Upload the package to PyPi:

```shell script
twine upload -r pypi ${AIRFLOW_REPO_ROOT}/dist/*.whl ${AIRFLOW_REPO_ROOT}/dist/*.tar.gz
```

* Verify that the packages are available under the links printed.

Copy links to updated packages, sort it alphabetically and save it on the side. You will need it for the announcement message.

* Again, confirm that the packages are available under the links printed.
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure why this is removed.
Or at least why we are changing the order of the steps in the release. I find it confusing.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not removed - it's moved up. And we are changing the order because we want to also publish the PyPI packages in SVN. So far we have not been doing it - because we did not need them in SVN (and it was a bit problematic as well because RC PyPI packages never went through our SVN and were never available there nor available in the history.

With "publish" workflow we want to make sure that packages are committed to SVN (including the PyPI packages) - that's why we are moving the "prepare" step before.

And since we eventually want to replace the "publish" step from manual to workflow, it makes sense to put the "manual" step next to the "workflow" test.

I think it could be also left below, but I found it cleaner if we have "publish" steps manual/workflow next to each other.

Copy link
Member Author

Choose a reason for hiding this comment

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

Let me know @eladkal if that explains it :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I will make the release according to instructions in this PR, that way we can change it on the fly then merge the final version that actually works. It will be easier to write questions/comments on the fly when the PR is up and change it as we progress

Copy link
Member Author

Choose a reason for hiding this comment

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

cool :) Sounds like a plan!

Comment on lines +510 to +513
# Move the artifacts to svn folder
mv ${AIRFLOW_REPO_ROOT}/dist/* .

# Add and commit
svn add *
Copy link
Contributor

Choose a reason for hiding this comment

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

This will not work.
We already moved from dist to release the packages to PyPi. When you get to this step dist is empty.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually not. We moved the "SVN" packages to SVN, the "PyPI" packages are created again few lines before:

breeze release-management prepare-provider-packages \
--version-suffix-for-pypi rc1 --package-format both PACKAGE PACKAGE ....

Those will have rc1 extension unlike SVN packages which might become the final release.

BTW. We should likely change names of those "types" of packages - previously for release candidates we commited only "SVN" packages to "SVN" and "PyPI" packages for PyPI - but now both will be committed to SVN.

So we likely need new names. Maybe:

  • "no-suffix" packages
  • "suffix" packages

?

@potiuk potiuk force-pushed the modify-providers-release-process-for-trusted-publishing-transition branch from fd14f8d to adeb27d Compare December 22, 2024 18:50
@potiuk potiuk force-pushed the modify-providers-release-process-for-trusted-publishing-transition branch from adeb27d to 44ed914 Compare December 22, 2024 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants