Skip to content

Conversation

@potiuk
Copy link
Member

@potiuk potiuk commented Oct 14, 2020

When extras are specifying when airflow is installed, this one triggers
installation of dependent packages. Each extra has a set of provider
packages that are needed by the extra and they will be installed
automatically if this extra is specified.

For now we do not add any version specificatiion, until we agree the
process in #11425 and then we should be able to implement an
automated way of getting information about cross-package
version dependencies.

Fixes: #11464


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

@potiuk potiuk force-pushed the extras-trigger-provider-download branch 2 times, most recently from 1ae7ebd to e5ffe28 Compare October 14, 2020 12:37
@potiuk
Copy link
Member Author

potiuk commented Oct 14, 2020

Hey @kaxil @ash - as mentioned on slack - I need to get that one merged in order to solve the Docker building problem that is now causing Kubernetes failures. Simply - when installing airflow[EXTRAS] I need to install also all the providers. This has not been possible yet because we do not have those packages in PyPI. This one is needed to fix #11490

This PR will likely fail in a few cases because Airflow will not find the right packages in PyPI to install:

Now the question is how we approach it:

  1. We can publish both airflow and provider's packages in PyPI. Though the result will be that the providers will be installed in the version from PyPI.

  2. We can try to implements some workaround to be able to install provider packages in Docker without having PyPI. This is possible, I can prepare packages before building docker, place them in 'docker-context-files' and install them from there using ``--find-links` option of PyPI which will install the packages from there. This is what I am currently leaning towards - in which case I will not need to have anything in PyPI (which for alphas might be a good idea).

  3. We could also just publish the providers in PyPI and do not install Airflow.

Another point is to solve the local installation (#11489) but I think this one will have to be installed differently because you for sure want to have local providers installed. This I am going to likely solve by a custom approach to not exclude packages in setup.py when installing locally (providing that I will be able to reliably detect the case that I am installing airflow from sources and not building the package which should be possible).

But I have to double check that one.

@github-actions
Copy link

The Workflow run is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.

@potiuk potiuk force-pushed the extras-trigger-provider-download branch from e5ffe28 to ec15b73 Compare October 14, 2020 16:11
@github-actions
Copy link

The Workflow run is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.

@potiuk potiuk force-pushed the extras-trigger-provider-download branch from ec15b73 to 64eeb06 Compare October 14, 2020 19:15
@potiuk
Copy link
Member Author

potiuk commented Oct 14, 2020

This one should work in principle now. I had hard time getting the version retrieval work, even if I manipulated the path, so if someone can make the check_extras_provider_packages work without the exception on reading version - I'd appreciate it , either I am to tired, or there is something fishy going on with the way how setup imports version.py which I do not understand.

@github-actions
Copy link

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport packages$,^Checks: Helm tests$,^Test OpenAPI*.

@potiuk potiuk force-pushed the extras-trigger-provider-download branch from 64eeb06 to 4e9bb8d Compare October 14, 2020 21:09
@github-actions
Copy link

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport packages$,^Checks: Helm tests$,^Test OpenAPI*.

@potiuk potiuk force-pushed the extras-trigger-provider-download branch from 4e9bb8d to 124b2ca Compare October 14, 2020 22:07
@github-actions
Copy link

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport packages$,^Checks: Helm tests$,^Test OpenAPI*.

@kaxil
Copy link
Member

kaxil commented Oct 15, 2020

Hey @kaxil @ash - as mentioned on slack - I need to get that one merged in order to solve the Docker building problem that is now causing Kubernetes failures. Simply - when installing airflow[EXTRAS] I need to install also all the providers. This has not been possible yet because we do not have those packages in PyPI. This one is needed to fix #11490

This PR will likely fail in a few cases because Airflow will not find the right packages in PyPI to install:

Now the question is how we approach it:

  1. We can publish both airflow and provider's packages in PyPI. Though the result will be that the providers will be installed in the version from PyPI.
  2. We can try to implements some workaround to be able to install provider packages in Docker without having PyPI. This is possible, I can prepare packages before building docker, place them in 'docker-context-files' and install them from there using ``--find-links` option of PyPI which will install the packages from there. This is what I am currently leaning towards - in which case I will not need to have anything in PyPI (which for alphas might be a good idea).
  3. We could also just publish the providers in PyPI and do not install Airflow.

Another point is to solve the local installation (#11489) but I think this one will have to be installed differently because you for sure want to have local providers installed. This I am going to likely solve by a custom approach to not exclude packages in setup.py when installing locally (providing that I will be able to reliably detect the case that I am installing airflow from sources and not building the package which should be possible).

But I have to double check that one.

I like (2)

@potiuk
Copy link
Member Author

potiuk commented Oct 15, 2020

I like (2)

Yep. That's what I am doing :)

Copy link
Member

@ashb ashb left a comment

Choose a reason for hiding this comment

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

I think we can get away without any .

Edit: I have no idea what I was going to say here anymore.

@potiuk
Copy link
Member Author

potiuk commented Oct 15, 2020

I think we can get away without any

Would love to hear how :).

I thought the same, but I am not so sure any more. I actually tested it and if you do pip install . now, the providers are not installed and if you are not in the "airflow" folder (, in pyhton 3.6+ is by default on python path) you won't be able to import providers.

Also when we add dependencies in extras, as you will see now - if you try to install airflow[google] - it will fail now because it won't find the apache-airflow-google-package. So I wonder how you would like to avoid those two problems.

We can of course do a "proper" sub-provider setup.py and keep separate setup.py per provider and split it and then have dependent package structure from airflow, but that would be a lot of maintenance, and especially if you want to manage common set of dependencies for the whole airflow, that woudl be quite complex. But If you would like to solve that this way - feel free :).

@potiuk potiuk force-pushed the extras-trigger-provider-download branch 2 times, most recently from 83094ff to 485bc68 Compare October 16, 2020 20:32
@github-actions
Copy link

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport packages$,^Checks: Helm tests$,^Test OpenAPI*.

@potiuk
Copy link
Member Author

potiuk commented Oct 17, 2020

All is ready now for merging - I think I managed to get it working in all cases as expected.

@potiuk potiuk added this to the Airflow 2.0.0-beta1 milestone Oct 18, 2020
@potiuk potiuk force-pushed the extras-trigger-provider-download branch 2 times, most recently from 286a80d to f51113d Compare October 22, 2020 14:35
@potiuk potiuk marked this pull request as draft October 22, 2020 14:36
@potiuk
Copy link
Member Author

potiuk commented Oct 22, 2020

Converted it to Draft as fhis one should be merged only after we release providers to PyPI.

@potiuk potiuk force-pushed the extras-trigger-provider-download branch from f51113d to e8eb429 Compare October 30, 2020 21:54
@potiuk potiuk marked this pull request as ready for review November 1, 2020 13:26
@potiuk
Copy link
Member Author

potiuk commented Nov 1, 2020

We should merge this just before we release our PyPI package for airflow beta 1

@potiuk potiuk force-pushed the extras-trigger-provider-download branch from e8eb429 to 1b0a3aa Compare November 1, 2020 19:04
@kaxil kaxil removed this from the Airflow 2.0.0-beta1 milestone Nov 2, 2020
@kaxil
Copy link
Member

kaxil commented Nov 4, 2020

Can you please rebase your PR on latest Master since we have applied Black and PyUpgrade on Master.

It will help if your squash your commits into single commit first so that there are less conflicts.

@potiuk potiuk force-pushed the extras-trigger-provider-download branch from 1b0a3aa to effc3e4 Compare November 9, 2020 14:39
@github-actions
Copy link

github-actions bot commented Nov 9, 2020

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

@potiuk potiuk force-pushed the extras-trigger-provider-download branch 2 times, most recently from d2e17ad to 35465a4 Compare November 9, 2020 17:57
@github-actions
Copy link

github-actions bot commented Nov 9, 2020

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

@github-actions
Copy link

github-actions bot commented Nov 9, 2020

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

@potiuk potiuk force-pushed the extras-trigger-provider-download branch from 35465a4 to 457decd Compare November 9, 2020 18:37
@potiuk
Copy link
Member Author

potiuk commented Nov 9, 2020

Looks that this one is good to go as well (there are some random cancels happening but I think they are because of GA problems not our own cancelling.)

When extras are specifying when airflow is installed, this one triggers
installation of dependent packages. Each extra has a set of provider
packages that are needed by the extra and they will be installed
automatically if this extra is specified.

For now we do not add any version specificatiion, until we agree the
process in apache#11425 and then we should be able to implement an
automated way of getting information about cross-package
version dependencies.

Fixes: apache#11464
@potiuk potiuk force-pushed the extras-trigger-provider-download branch from 457decd to 1189134 Compare November 9, 2020 20:36
@ashb ashb merged commit ea27f90 into apache:master Nov 9, 2020
potiuk added a commit to PolideaInternal/airflow that referenced this pull request Nov 10, 2020
The apache#11526 was badly rebased just before beta1 relase and few
lines installing the providers were lost.

This PR restores those lines.

Fixes: apache#12231
potiuk added a commit that referenced this pull request Nov 10, 2020
The #11526 was badly rebased just before beta1 relase and few
lines installing the providers were lost.

This PR restores those lines.

Fixes: #12231
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.

Make extras install corresponding provider packages.

3 participants