Fix sdist verification in CI after we standardized packaging #37406
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Airflow Sdist packages have been broken by apache#37340 and fixed by 37388, but we have not noticed it because CI check for sdist packages has been broken since apache#36537 where we standardized naming of the sdist packages to follow modern syntax (and we silently skipped installation because no providers were found),. This PR fixes it: * changes the naming format expected to follow the new standard * treats "no providers found as error" The "no providers" as success was useful at some point of time when we run sdist as part of regular PRs and some PRs resulted in "no providers changed" condition, however sdist verification only happens now in canary build (so all providers are affected) as well as we have if condition in the job itself to skip the step of installation if there are no providers.
potiuk
requested review from
Taragolis,
pierrejeambrun,
amoghrajesh and
hussein-awala
hussein-awala
approved these changes
Feb 13, 2024
potiuk
added a commit
that referenced
this pull request
Feb 14, 2024
sunank200
pushed a commit
to astronomer/airflow
that referenced
this pull request
Feb 21, 2024
…37406) Airflow Sdist packages have been broken by apache#37340 and fixed by 37388, but we have not noticed it because CI check for sdist packages has been broken since apache#36537 where we standardized naming of the sdist packages to follow modern syntax (and we silently skipped installation because no providers were found),. This PR fixes it: * changes the naming format expected to follow the new standard * treats "no providers found as error" The "no providers" as success was useful at some point of time when we run sdist as part of regular PRs and some PRs resulted in "no providers changed" condition, however sdist verification only happens now in canary build (so all providers are affected) as well as we have if condition in the job itself to skip the step of installation if there are no providers.
sunank200
pushed a commit
to astronomer/airflow
that referenced
this pull request
Feb 21, 2024
Follow up after apache#37406 - we could not test this change as there was no way to test canary builds but as the capability has been added in apache#37408 we can fix the failure and actually test it in a "canary" PR.
abhishekbhakat
pushed a commit
to abhishekbhakat/my_airflow
that referenced
this pull request
Mar 5, 2024
…37406) Airflow Sdist packages have been broken by apache#37340 and fixed by 37388, but we have not noticed it because CI check for sdist packages has been broken since apache#36537 where we standardized naming of the sdist packages to follow modern syntax (and we silently skipped installation because no providers were found),. This PR fixes it: * changes the naming format expected to follow the new standard * treats "no providers found as error" The "no providers" as success was useful at some point of time when we run sdist as part of regular PRs and some PRs resulted in "no providers changed" condition, however sdist verification only happens now in canary build (so all providers are affected) as well as we have if condition in the job itself to skip the step of installation if there are no providers.
abhishekbhakat
pushed a commit
to abhishekbhakat/my_airflow
that referenced
this pull request
Mar 5, 2024
Follow up after apache#37406 - we could not test this change as there was no way to test canary builds but as the capability has been added in apache#37408 we can fix the failure and actually test it in a "canary" PR.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Airflow Sdist packages have been broken by #37340 and fixed by 37388, but we have not noticed it because CI check for sdist packages has been broken since #36537 where we standardized naming of the sdist packages to follow modern syntax (and we silently skipped installation because no providers were found),.
This PR fixes it:
The "no providers" as success was useful at some point of time when we run sdist as part of regular PRs and some PRs resulted in "no providers changed" condition, however sdist verification only happens now in canary build (so all providers are affected) as well as we have if condition in the job itself to skip the step of installation if there are no providers.
^ 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.rstor{issue_number}.significant.rst, in newsfragments.