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

Add description on the vendoring process we use #22204

Merged
merged 1 commit into from
Mar 20, 2022

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Mar 12, 2022

We need to vendor in cgroupspy library in order to make Airflow
compatible with Python 3.10 (see #22050) so this is the right time
to make our vendoring process more organized.

I based in parts on the readme described by bleach package.


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, 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 UPDATING.md.

@uranusjr
Copy link
Member

Some ideas from how pip does this. pip fully automates the vendoring-patching process so a new version can be pulled immediately if needed. It uses vendoring for this. It takes a requirements file (a la requiurements.txt) and diff files for patches. When invoked, it downloads and extracts the package and applies the patches in the vendor directory (specified in pyproject.toml).

Not sure if Airflow needs this much infrastructure (we only vendor two? dependencies and don’t generally upgrade the existing one), but could be useful if we want to go this route.

@potiuk
Copy link
Member Author

potiuk commented Mar 13, 2022

Not sure if Airflow needs this much infrastructure (we only vendor two? dependencies and don’t generally upgrade the existing one), but could be useful if we want to go this route.

Yeah. I considered it too, but let's say we might want to do it when we see if it is needed. For now we have 0 vendored-in dependencies and and the aim is to remove them asap - likely with next release (I am going to submit a PR to fix it in cgroupspy in a moment).

@potiuk
Copy link
Member Author

potiuk commented Mar 13, 2022

The Fix to cgroupspy submitted: cloudsigma/cgroupspy#14 - but the last change was 7 months ago, so I am not sure how fast it might get merged/released.

@potiuk
Copy link
Member Author

potiuk commented Mar 13, 2022

Yeah. I considered it too, but let's say we might want to do it when we see if it is needed. For now we have 0 vendored-in dependencies and and the aim is to remove them asap - likely with next release (I am going to submit a PR to fix it in cgroupspy in a moment).

I tihnk when we have more than one vendored in dependencies with significant changes and active development of the vendored-library (and when we see that such vendoring-in might be needed for at least few releases - we should consider automating it - in the case we have now with cgroupspy - I can't imagine they release new version without accepting the single abc import change, so I think any automation around it is a waste :)

@potiuk potiuk force-pushed the add-description-of-vendoring-process branch from 335b4db to 2be7857 Compare March 13, 2022 15:03
@potiuk
Copy link
Member Author

potiuk commented Mar 13, 2022

First in-line to get finally Python 3.10 support :)

@potiuk potiuk force-pushed the add-description-of-vendoring-process branch from 2be7857 to b7d4037 Compare March 14, 2022 18:22
@potiuk
Copy link
Member Author

potiuk commented Mar 14, 2022

Hey @kaxil I cherry-picked your process description fixes from #22209 and added more description on the "no-compile".

@potiuk potiuk closed this Mar 14, 2022
@potiuk potiuk reopened this Mar 14, 2022
@potiuk potiuk force-pushed the add-description-of-vendoring-process branch from b7d4037 to 83741e3 Compare March 14, 2022 21:40
@potiuk
Copy link
Member Author

potiuk commented Mar 15, 2022

Hey @kaxil -> can we merge it ? I hope to start merging the series of PRs: #22204, #22206, #22207, #22208, #22209 leading to Python 3.10 compatibility on Monay (hopefully) when I releae all the provider packages.

@potiuk
Copy link
Member Author

potiuk commented Mar 17, 2022

🙏

We need to vendor in cgroupspy library in order to make Airflow
compatible with Python 3.10 (see apache#22050) so this is the right time
to make our vendoring process more organized.

I based in parts on the readme described by `bleach` package.
@potiuk potiuk force-pushed the add-description-of-vendoring-process branch from 83741e3 to 2ec4ddb Compare March 19, 2022 21:06
@potiuk
Copy link
Member Author

potiuk commented Mar 19, 2022

I rebased on top of latest changes - I need those #22204, #22206, #22207, #22208, #22209 to complete Python 3.10 support

Looking for approvals on all of those (they are slal small)

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Mar 19, 2022
@github-actions
Copy link

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

@potiuk potiuk merged commit 9372aab into apache:main Mar 20, 2022
@potiuk potiuk deleted the add-description-of-vendoring-process branch March 20, 2022 09:54
@ephraimbuddy ephraimbuddy added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Apr 8, 2022
@potiuk potiuk restored the add-description-of-vendoring-process branch April 26, 2022 20:52
@potiuk potiuk deleted the add-description-of-vendoring-process branch July 29, 2022 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) full tests needed We need to run full set of tests for this PR to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants