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

Newest providers incorrectly include gitpython and wheel in install_requires #22380

Closed
1 of 2 tasks
xylar opened this issue Mar 20, 2022 · 14 comments · Fixed by #22382
Closed
1 of 2 tasks

Newest providers incorrectly include gitpython and wheel in install_requires #22380

xylar opened this issue Mar 20, 2022 · 14 comments · Fixed by #22382
Assignees
Labels
area:providers kind:bug This is a clearly a bug

Comments

@xylar
Copy link
Contributor

xylar commented Mar 20, 2022

Apache Airflow Provider(s)

ftp, openfaas, sqlite

Versions of Apache Airflow Providers

I am the maintainer of the Airflow Providers on conda-forge. The providers I listed above are the first 3 I have looked at but I believe all are affected. These are the new releases (as of yesterday) of all providers.

Apache Airflow version

2.2.4 (latest released)

Operating System

Linux (Azure CI)

Deployment

Other Docker-based deployment

Deployment details

This is on conda-forge Azure CI.

What happened

All providers I have looked at (and I suspect all providers) now have gitpython and wheel in their install_requires:

From apache-airflow-providers-ftp-2.1.1.tar.gz:

install_requires = 
	
	gitpython
	wheel

I believe these requirements are incorrect (neither should be needed at install time) and this will make maintaining these packages on conda-forge an absolute nightmare! (It's already a serious challenge because I get a PR to update each time each provider gets updated.)

What you think should happen instead

These install requirements should be removed.

How to reproduce

Open any of the newly released providers from pypi and look at setup.cfg.

Anything else

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@xylar xylar added area:providers kind:bug This is a clearly a bug labels Mar 20, 2022
@boring-cyborg
Copy link

boring-cyborg bot commented Mar 20, 2022

Thanks for opening your first issue here! Be sure to follow the issue template!

@xylar
Copy link
Contributor Author

xylar commented Mar 20, 2022

I believe this was added in #22226, perhaps by mistake?

@eladkal
Copy link
Contributor

eladkal commented Mar 20, 2022

cc @potiuk

@potiuk potiuk self-assigned this Mar 20, 2022
@potiuk
Copy link
Member

potiuk commented Mar 20, 2022

Yep. This is my mistake indeed. I will have to rerelease all of the providers and yank all of them I am afraid

@xylar
Copy link
Contributor Author

xylar commented Mar 20, 2022

@potiuk, no worries on my side. I know I sounded pretty annoyed above. But this was the excuse I needed to automate my update process on conda-forge.

Sorry that it's going to be a bit of work for you, though. Thanks for taking care of it!

@potiuk
Copy link
Member

potiuk commented Mar 20, 2022

Sorry that it's going to be a bit of work for you, though. Thanks for taking care of it!

No - no worries at all - thanks for reporting it so quickly @xylar actually. It slipeed my automation tests (for good reasons) and I have the releases mostly automated, so the work from my side is not as big as it might seem (highly recommend to add the automation indeed :).

@potiuk
Copy link
Member

potiuk commented Mar 20, 2022

BTW. Learning for myself - DO split changes and semi-automated documentation generation. This way the change could have been detected at code review time. The #22226 change was far too big to catch it.

potiuk added a commit to potiuk/airflow that referenced this issue Mar 20, 2022
The TroveClassifiers change apache#22226 - by mistake - added the
gitpython and wheel for all providers.

This could have been avoided (and noticed) if we split the change
from doc generation. So as a learning I separate a fix to only
fix the problem.

Fix apache#22380
potiuk added a commit that referenced this issue Mar 20, 2022
The TroveClassifiers change #22226 - by mistake - added the
gitpython and wheel for all providers.

This could have been avoided (and noticed) if we split the change
from doc generation. So as a learning I separate a fix to only
fix the problem.

Fix #22380
@potiuk
Copy link
Member

potiuk commented Mar 24, 2022

FYI in case you did not notice @xylar - we are voting on the packages with the fix Discussion here: https://lists.apache.org/thread/g990cb3vqkzq7jr3yvldy253wtt8vhnq and "test issue" in #22480.

Would be great if you could confirm that the RC packages look good for you as well (few of them that contains some other fixed are mentioned and linked from #22480).

@potiuk
Copy link
Member

potiuk commented Mar 24, 2022

BTW. I compared (for example) https://pypi.org/pypi/apache-airflow-providers-docker/2.5.1/json with https://pypi.org/pypi/apache-airflow-providers-docker/2.5.2rc1/json so it should be fine - but I think with your conda stuff you can check more :)

@xylar
Copy link
Contributor Author

xylar commented Mar 24, 2022

Thank you! I agree that the the dependencies in apache-airflow-providers-docker 2.5.2rc1 look good. If the other providers are similar, we'll be in great shape!

@xylar
Copy link
Contributor Author

xylar commented Mar 24, 2022

Yep, I took a look at several more and they also look good.

@potiuk
Copy link
Member

potiuk commented Mar 24, 2022

Yep, I took a look at several more and they also look good.

Cool. Thanks for sparing another pair of eyes :)

@potiuk
Copy link
Member

potiuk commented Mar 26, 2022

New providers released. They are all yours @xylar

@xylar
Copy link
Contributor Author

xylar commented Mar 26, 2022

Thanks again. Most new providers should be should now be available from conda-forge. There were a few newer ones that I hadn't added recipes for yet. They're awaiting review (or review of missing dependencies).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers kind:bug This is a clearly a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants