Skip to content

Fixing remove_old_releases.py script to handle an edge case#35942

Merged
potiuk merged 1 commit into
apache:mainfrom
amoghrajesh:fixOldReleaseScript
Nov 29, 2023
Merged

Fixing remove_old_releases.py script to handle an edge case#35942
potiuk merged 1 commit into
apache:mainfrom
amoghrajesh:fixOldReleaseScript

Conversation

@amoghrajesh

Copy link
Copy Markdown
Contributor

There was a bug in the remove_old_releases.py where due to change in the provider tarball format from _ to -, the script failed to detect the provider packages and lead to the script failing.

For examples, apache-airflow-providers-amazon-8.11.0.tar.gz and apache_airflow_providers_amazon-8.11.0.tar.gz
Before changes:
VersionedFile(base='apache-airflow-providers-amazon-', version='8.11.0', suffix='.tar.gz', type='apache-airflow-providers-amazon-.tar.gz', comparable_version=<Version('8.11.0')>)
VersionedFile(base='apache_airflow_providers_amazon-', version='8.12.0', suffix='.tar.gz', type='apache_airflow_providers_amazon-.tar.gz', comparable_version=<Version('8.12.0')>)

After changes:
VersionedFile(base='apache-airflow-providers-amazon-', version='8.11.0', suffix='.tar.gz', type='apache-airflow-providers-amazon-.tar.gz', comparable_version=<Version('8.11.0')>)
VersionedFile(base='apache-airflow-providers-amazon-', version='8.11.0', suffix='.tar.gz', type='apache-airflow-providers-amazon-.tar.gz', comparable_version=<Version('8.11.0')>)


^ 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.

@jscheffl jscheffl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Unable to execute to test this (or tell me which README to follow) but code looks OK.
Does it make sense to add a hint "why" this code is there? For somebody scratching the head in the future? (git log is your fried but might be lost if code is re-factored)

@potiuk potiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @amoghrajesh. Cool. It works!

I just run it on the "providers" release https://dist.apache.org/repos/dist/release/airflow/providers/ - it has built in "dry-run" mode - when run without --execute flag it will show what it is going to do so I was sure it was working fine :).

So I just run it and removed old releases.

The nice thing about it is that once we have new release of packages (which wil happen early December as we will be bumping min-version) - the whl/sdist packages will finally be next to each other as they will follow same patterns. finally we have consistency (as there was a discrepancy between the naming of wheel/setuptools) - which is another good side-effect of switching to a modern packaging tool like flit.

@potiuk

potiuk commented Nov 29, 2023

Copy link
Copy Markdown
Member

the "generate constraint" failure is unrelatesd and hopefully fixed/mitigated in #35945

@potiuk potiuk merged commit 4b75a88 into apache:main Nov 29, 2023
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.

3 participants