You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/10/14 12:34:27 UTC
[GitHub] [airflow] potiuk opened a new pull request #11526: Adds automated installation of dependent packages
potiuk opened a new pull request #11526:
URL: https://github.com/apache/airflow/pull/11526
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
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#issuecomment-708617180
[The Workflow run](https://github.com/apache/airflow/actions/runs/307186976) 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*.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#issuecomment-708676394
[The Workflow run](https://github.com/apache/airflow/actions/runs/307373711) 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*.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#issuecomment-711043585
All is ready now for merging - I think I managed to get it working in all cases as expected.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#issuecomment-709247235
> 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:
>
> * when installing Docker (#11490)
> * when installing Airflow locally from sources (#11489) with editable (likely not) or just regular local install with 'pip 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)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#issuecomment-724193045
[The Workflow run](https://github.com/apache/airflow/actions/runs/354495356) 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*.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#discussion_r510071759
##########
File path: setup.py
##########
@@ -34,9 +34,12 @@
# Kept manually in sync with airflow.__version__
spec = util.spec_from_file_location("airflow.version", os.path.join('airflow', 'version.py')) # noqa
-mod = util.module_from_spec(spec)
-spec.loader.exec_module(mod) # type: ignore
-version = mod.version # type: ignore
+try:
+ mod = util.module_from_spec(spec)
+ spec.loader.exec_module(mod) # type: ignore
+ version = mod.version # type: ignore
+except Exception: # pylint: disable=broad-except
+ version = None
Review comment:
Can you add a comment please in the line above try as why this `try..except` block is needed
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#issuecomment-708380382
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:
- when installing Docker (#11490)
- when installing Airflow locally from sources (#11489) with editable (likely not) or just regular local install with 'pip 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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#discussion_r510211556
##########
File path: setup.py
##########
@@ -34,9 +34,12 @@
# Kept manually in sync with airflow.__version__
spec = util.spec_from_file_location("airflow.version", os.path.join('airflow', 'version.py')) # noqa
-mod = util.module_from_spec(spec)
-spec.loader.exec_module(mod) # type: ignore
-version = mod.version # type: ignore
+try:
+ mod = util.module_from_spec(spec)
+ spec.loader.exec_module(mod) # type: ignore
+ version = mod.version # type: ignore
+except Exception: # pylint: disable=broad-except
+ version = None
Review comment:
Sure! Done.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#issuecomment-724065916
[The Workflow run](https://github.com/apache/airflow/actions/runs/354140063) 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*.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#issuecomment-709323347
> 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 :).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#issuecomment-710664002
[The Workflow run](https://github.com/apache/airflow/actions/runs/311489636) 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*.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#discussion_r505570130
##########
File path: Dockerfile
##########
@@ -206,6 +206,9 @@ ENV AIRFLOW_LOCAL_PIP_WHEELS=${AIRFLOW_LOCAL_PIP_WHEELS}
ARG SLUGIFY_USES_TEXT_UNIDECODE=""
ENV SLUGIFY_USES_TEXT_UNIDECODE=${SLUGIFY_USES_TEXT_UNIDECODE}
+ARG INSTALL_ROVIDERS_FROM_SOURCES="true"
Review comment:
> There is a typo too btw `INSTALL_ROVIDERS_FROM_SOURCES` instead of `INSTALL_PROVIDERS_FROM_SOURCES`
Ah yeah!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#issuecomment-709323347
> 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 quiet complex. But If you would like to solve that this way - feel free :).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#discussion_r510213598
##########
File path: setup.py
##########
@@ -741,13 +839,27 @@ def is_package_excluded(package: str, exclusion_list: List[str]):
]
+def get_provider_package_from_package_id(package_id: str):
+ """
+ Builds the name of provider package out of the package id provided/
+
+ :param package_id: id of the package (like amazon or microsoft.azure)
+ :return: full name of package in PyPI
+ """
+ package_suffix = package_id.replace(".", "-")
+ return f"apache-airflow-providers-{package_suffix}"
+
+
def do_setup():
"""Perform the Airflow package setup."""
- install_providers_from_sources = os.getenv('INSTALL_PROVIDERS_FROM_SOURCES')
- exclude_patterns = \
- [] if install_providers_from_sources and install_providers_from_sources == 'true' \
- else ['airflow.providers', 'airflow.providers.*']
+ install_providers_from_sources = os.getenv('INSTALL_PROVIDERS_FROM_SOURCES') == 'true' or False
Review comment:
actually even
```
install_providers_from_sources = os.getenv('INSTALL_PROVIDERS_FROM_SOURCES') == 'true'
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#discussion_r504747187
##########
File path: setup.py
##########
@@ -637,6 +638,95 @@ def write_version(filename: str = os.path.join(*[my_dir, "airflow", "git_version
'yandexcloud': yandexcloud,
}
+EXTRAS_PROVIDERS_PACKAGES: Dict[str, Iterable[str]] = {
+ 'all_dbs': [
+ "apache.cassandra", "apache.druid", "apache.hdfs", "apache.hive", "apache.pinot",
+ "cloudant", "exasol",
+ "mongo", "microsoft.mssql", "mysql", "postgres", "presto", "vertica"
+ ],
+ 'amazon': ["amazon"],
+ 'apache.atlas': [],
+ 'apache.beam': [],
+ "apache.cassandra": ["apache.cassandra"],
+ "apache.druid": ["apache.druid"],
+ "apache.hdfs": ["apache.hdfs"],
+ "apache.hive": ["apache.hive"],
+ "apache.kylin": ["apache.kylin"],
+ "apache.pinot": ["apache.pinot"],
+ "apache.webhdfs": ["apache.hdfs"],
+ 'async': [],
+ 'atlas': [], # TODO: remove this in Airflow 2.1
+ 'aws': ["amazon"], # TODO: remove this in Airflow 2.1
+ 'azure': ["microsoft.azure"], # TODO: remove this in Airflow 2.1
+ 'cassandra': ["apache.cassandra"], # TODO: remove this in Airflow 2.1
+ 'celery': ["celery"],
+ 'cgroups': [],
+ 'cloudant': ["cloudant"],
+ 'cncf.kubernetes': ["cncf.kubernetes"],
+ 'dask': ["dask"],
+ 'databricks': ["databricks"],
+ 'datadog': ["datadog"],
+ 'devel': ["cncf.kubernetes", "mysql"],
+ 'devel_hadoop': ["apache.hdfs", "apache.hive", "presto"],
+ 'doc': [],
+ 'docker': ["docker"],
+ 'druid': ["apache.druid"], # TODO: remove this in Airflow 2.1
+ 'elasticsearch': ["elasticsearch"],
+ 'exasol': ["exasol"],
+ 'facebook': ["facebook"],
+ 'gcp': ["google"], # TODO: remove this in Airflow 2.1
+ 'gcp_api': ["google"], # TODO: remove this in Airflow 2.1
+ 'github_enterprise': [],
+ 'google': ["google"],
+ 'google_auth': [],
+ 'grpc': ["grpc"],
+ 'hashicorp': ["hashicorp"],
+ 'hdfs': ["apache.hdfs"], # TODO: remove this in Airflow 2.1
+ 'hive': ["apache.hive"], # TODO: remove this in Airflow 2.1
+ 'jdbc': ["jdbc"],
+ 'jira': ["jira"],
+ 'kerberos': [],
+ 'kubernetes': ["cncf.kubernetes"], # TODO: remove this in Airflow 2.1
+ 'ldap': [],
Review comment:
Why bother including the empty ones in this list?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#issuecomment-708702196
[The Workflow run](https://github.com/apache/airflow/actions/runs/307465552) 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*.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#discussion_r505563496
##########
File path: Dockerfile
##########
@@ -206,6 +206,9 @@ ENV AIRFLOW_LOCAL_PIP_WHEELS=${AIRFLOW_LOCAL_PIP_WHEELS}
ARG SLUGIFY_USES_TEXT_UNIDECODE=""
ENV SLUGIFY_USES_TEXT_UNIDECODE=${SLUGIFY_USES_TEXT_UNIDECODE}
+ARG INSTALL_ROVIDERS_FROM_SOURCES="true"
Review comment:
This is exactly what I tried to start a discussion about here. Happy to hear your opinions on that.
For now, I deliberately chose it to default to "true" by design. But I am open to discuss it and happy to hear your opinion/arguments why it should not be if you think otherwise.
By default, when you are using production Dockerfile, we are installing Airflow from the sources. Actually, you even can't use the Dockerfile outside of sources because (as it is with most of the Dockerfiles) you need scripts, some extra files etc., and generally, the docker context where some relevant scripts/potentially binaries are stored. I think this is something that people will do rarely (we did it for example when developing the POC with the customer recently but then we were comfortable with pulling the latest version of airflow) - and eventually, we switched to installing airflow from binary pip packages in the image.
So the default behavior for production Dockerfile when you run 'docker build .' is that it builds airflow from the sources. This is also consistent with what we are releasing as .src.tgz - you only need that source package (and all the apt/pip) to rebuild a fully functional Apache Airflow image in the very exact version the sources were released with.
So part of my proposal here - for anyone building the Docker Image from the sources(- is that they will still get all set of providers. This is backward compatible, convenient, and generally should only be used by developers.
There is the second build mode where you use PIP packages (--build-arg INSTALL_AIRFLOW_VERSION=1.10.12) for example. And in this mode Airflow is installed from PyPI with the version specified. You still need Airflow sources for that (because you need the scripts) but I am thinking about a way to not require sources to build a custom image. Happy to hear some suggestions here as well.
Another way of building the image is that you can use .whl packages placed in `docker-context-files` or even directly a version tagged in GitHub repository of Airflow (--build-arg INSTALL_AIRLFOW_REFERENCE=master).
In all those cases besides default building from sources, the INSTALL_PROVIDER_FROM_SOURCES has no effect and you only install the core package. Unless you specify [extra] (when this PR gets merged). In such case, those extra packages will be installed from PyPI or from other sources that you configure in .piprc - also configurable now. For example, you could configure .piprc to point to `/docker-context-files` as "source" of packages and place all the .whl provider packages there and then installing airflow[google] will get the packages from those wheels placed in the `docker-context-files` folder rather than PyPI. We have few flags that together eliminate entirely the needs of reaching out to any external server (besides APT mirrors) during the build. There are some details and examples in `docs/production-deployments.rst`.
This is all highly optimized in the current way image is build, because those `docker-context-files` are only copied to the "build" segment of the image and never land in the final image. This is also a very controlled, highly secure, perfectly sound and maintainable way of building Airflow image. It is forward-compatible with the future version of Dockerfiles we might have) from vetted "local" binary sources - it does not require the customers to modify the Dockerfile and keep their own copy. This BTW is something that at least two of our huge customers either do in production now or will do very soon - switch to use this Dockerfile and use it rather than provide their own. So this is a highly requested feature from big customers - their security teams expect a buildable Dockerfile and they want to make sure it will work in a future version of Airflo without some modifications that have to be cherry-picked in new versions.
As soon as we reach 2.0 /1.10.13 and I apply for the "official" status, I will make sure any future changes will be backward-compatible BTW.
So for me, this is by design to have `INSTALL_PROVIDERS_FROM_SOURCES` "true" by default. But If you have some scenarios/concerns about it, I am happy to hear them.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb merged pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
ashb merged pull request #11526:
URL: https://github.com/apache/airflow/pull/11526
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#discussion_r505526005
##########
File path: Dockerfile
##########
@@ -206,6 +206,9 @@ ENV AIRFLOW_LOCAL_PIP_WHEELS=${AIRFLOW_LOCAL_PIP_WHEELS}
ARG SLUGIFY_USES_TEXT_UNIDECODE=""
ENV SLUGIFY_USES_TEXT_UNIDECODE=${SLUGIFY_USES_TEXT_UNIDECODE}
+ARG INSTALL_ROVIDERS_FROM_SOURCES="true"
Review comment:
There is a typo too btw `INSTALL_ROVIDERS_FROM_SOURCES` instead of `INSTALL_PROVIDERS_FROM_SOURCES`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#discussion_r504883070
##########
File path: setup.py
##########
@@ -637,6 +638,95 @@ def write_version(filename: str = os.path.join(*[my_dir, "airflow", "git_version
'yandexcloud': yandexcloud,
}
+EXTRAS_PROVIDERS_PACKAGES: Dict[str, Iterable[str]] = {
+ 'all_dbs': [
+ "apache.cassandra", "apache.druid", "apache.hdfs", "apache.hive", "apache.pinot",
+ "cloudant", "exasol",
+ "mongo", "microsoft.mssql", "mysql", "postgres", "presto", "vertica"
+ ],
+ 'amazon': ["amazon"],
+ 'apache.atlas': [],
+ 'apache.beam': [],
+ "apache.cassandra": ["apache.cassandra"],
+ "apache.druid": ["apache.druid"],
+ "apache.hdfs": ["apache.hdfs"],
+ "apache.hive": ["apache.hive"],
+ "apache.kylin": ["apache.kylin"],
+ "apache.pinot": ["apache.pinot"],
+ "apache.webhdfs": ["apache.hdfs"],
+ 'async': [],
+ 'atlas': [], # TODO: remove this in Airflow 2.1
+ 'aws': ["amazon"], # TODO: remove this in Airflow 2.1
+ 'azure': ["microsoft.azure"], # TODO: remove this in Airflow 2.1
+ 'cassandra': ["apache.cassandra"], # TODO: remove this in Airflow 2.1
+ 'celery': ["celery"],
+ 'cgroups': [],
+ 'cloudant': ["cloudant"],
+ 'cncf.kubernetes': ["cncf.kubernetes"],
+ 'dask': ["dask"],
+ 'databricks': ["databricks"],
+ 'datadog': ["datadog"],
+ 'devel': ["cncf.kubernetes", "mysql"],
+ 'devel_hadoop': ["apache.hdfs", "apache.hive", "presto"],
+ 'doc': [],
+ 'docker': ["docker"],
+ 'druid': ["apache.druid"], # TODO: remove this in Airflow 2.1
+ 'elasticsearch': ["elasticsearch"],
+ 'exasol': ["exasol"],
+ 'facebook': ["facebook"],
+ 'gcp': ["google"], # TODO: remove this in Airflow 2.1
+ 'gcp_api': ["google"], # TODO: remove this in Airflow 2.1
+ 'github_enterprise': [],
+ 'google': ["google"],
+ 'google_auth': [],
+ 'grpc': ["grpc"],
+ 'hashicorp': ["hashicorp"],
+ 'hdfs': ["apache.hdfs"], # TODO: remove this in Airflow 2.1
+ 'hive': ["apache.hive"], # TODO: remove this in Airflow 2.1
+ 'jdbc': ["jdbc"],
+ 'jira': ["jira"],
+ 'kerberos': [],
+ 'kubernetes': ["cncf.kubernetes"], # TODO: remove this in Airflow 2.1
+ 'ldap': [],
Review comment:
BTW. In the future we will synchronise all extras and providers names - when we remove all the TODOs in 2.1 we will be able to simply make sure that we have all the keys defined in all the dicts here, but this should only happen then. For now I am adding a pre-commit check now to make sure that all extras are defined in EXTRAS_PROVIDERS_PACKAGES.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#issuecomment-721445824
Can you please rebase your PR on latest Master since we have applied [Black](https://github.com/apache/airflow/commit/4e8f9cc8d02b29c325b8a5a76b4837671bdf5f68) and [PyUpgrade](https://github.com/apache/airflow/commit/8c42cf1b00c90f0d7f11b8a3a455381de8e003c5) on Master.
It will help if your squash your commits into single commit first so that there are less conflicts.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#discussion_r505513770
##########
File path: Dockerfile
##########
@@ -206,6 +206,9 @@ ENV AIRFLOW_LOCAL_PIP_WHEELS=${AIRFLOW_LOCAL_PIP_WHEELS}
ARG SLUGIFY_USES_TEXT_UNIDECODE=""
ENV SLUGIFY_USES_TEXT_UNIDECODE=${SLUGIFY_USES_TEXT_UNIDECODE}
+ARG INSTALL_ROVIDERS_FROM_SOURCES="true"
Review comment:
This Dockerfile is used for prod images isn't it? This should default to False then, no?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#issuecomment-709277615
> I like (2)
Yep. That's what I am doing :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#discussion_r510073225
##########
File path: setup.py
##########
@@ -741,13 +839,27 @@ def is_package_excluded(package: str, exclusion_list: List[str]):
]
+def get_provider_package_from_package_id(package_id: str):
+ """
+ Builds the name of provider package out of the package id provided/
+
+ :param package_id: id of the package (like amazon or microsoft.azure)
+ :return: full name of package in PyPI
+ """
+ package_suffix = package_id.replace(".", "-")
+ return f"apache-airflow-providers-{package_suffix}"
+
+
def do_setup():
"""Perform the Airflow package setup."""
- install_providers_from_sources = os.getenv('INSTALL_PROVIDERS_FROM_SOURCES')
- exclude_patterns = \
- [] if install_providers_from_sources and install_providers_from_sources == 'true' \
- else ['airflow.providers', 'airflow.providers.*']
+ install_providers_from_sources = os.getenv('INSTALL_PROVIDERS_FROM_SOURCES') == 'true' or False
Review comment:
```suggestion
install_providers_from_sources = bool(os.getenv('INSTALL_PROVIDERS_FROM_SOURCES') == 'true')
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#issuecomment-714539316
Converted it to Draft as fhis one should be merged only after we release providers to PyPI.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#issuecomment-708608557
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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#discussion_r504879545
##########
File path: setup.py
##########
@@ -637,6 +638,95 @@ def write_version(filename: str = os.path.join(*[my_dir, "airflow", "git_version
'yandexcloud': yandexcloud,
}
+EXTRAS_PROVIDERS_PACKAGES: Dict[str, Iterable[str]] = {
+ 'all_dbs': [
+ "apache.cassandra", "apache.druid", "apache.hdfs", "apache.hive", "apache.pinot",
+ "cloudant", "exasol",
+ "mongo", "microsoft.mssql", "mysql", "postgres", "presto", "vertica"
+ ],
+ 'amazon': ["amazon"],
+ 'apache.atlas': [],
+ 'apache.beam': [],
+ "apache.cassandra": ["apache.cassandra"],
+ "apache.druid": ["apache.druid"],
+ "apache.hdfs": ["apache.hdfs"],
+ "apache.hive": ["apache.hive"],
+ "apache.kylin": ["apache.kylin"],
+ "apache.pinot": ["apache.pinot"],
+ "apache.webhdfs": ["apache.hdfs"],
+ 'async': [],
+ 'atlas': [], # TODO: remove this in Airflow 2.1
+ 'aws': ["amazon"], # TODO: remove this in Airflow 2.1
+ 'azure': ["microsoft.azure"], # TODO: remove this in Airflow 2.1
+ 'cassandra': ["apache.cassandra"], # TODO: remove this in Airflow 2.1
+ 'celery': ["celery"],
+ 'cgroups': [],
+ 'cloudant': ["cloudant"],
+ 'cncf.kubernetes': ["cncf.kubernetes"],
+ 'dask': ["dask"],
+ 'databricks': ["databricks"],
+ 'datadog': ["datadog"],
+ 'devel': ["cncf.kubernetes", "mysql"],
+ 'devel_hadoop': ["apache.hdfs", "apache.hive", "presto"],
+ 'doc': [],
+ 'docker': ["docker"],
+ 'druid': ["apache.druid"], # TODO: remove this in Airflow 2.1
+ 'elasticsearch': ["elasticsearch"],
+ 'exasol': ["exasol"],
+ 'facebook': ["facebook"],
+ 'gcp': ["google"], # TODO: remove this in Airflow 2.1
+ 'gcp_api': ["google"], # TODO: remove this in Airflow 2.1
+ 'github_enterprise': [],
+ 'google': ["google"],
+ 'google_auth': [],
+ 'grpc': ["grpc"],
+ 'hashicorp': ["hashicorp"],
+ 'hdfs': ["apache.hdfs"], # TODO: remove this in Airflow 2.1
+ 'hive': ["apache.hive"], # TODO: remove this in Airflow 2.1
+ 'jdbc': ["jdbc"],
+ 'jira': ["jira"],
+ 'kerberos': [],
+ 'kubernetes': ["cncf.kubernetes"], # TODO: remove this in Airflow 2.1
+ 'ldap': [],
Review comment:
For the very reason why https://github.com/apache/airflow/runs/1253789707?check_suite_focus=true#step:9:3844 failed.
I want to explicitly list all extras here, because we can miss some of those extras. I missed "devel_ci" and "all" for example because they are added few pages down.
If I remove the empty ones, I'd have to implement the logic "If missing - do not add any providers". But "missing" in this case has a different semantics: "someone has forgotten to add it" rather than "explicitly decided that this extras does not need any provider".
In this case explicit is far better than explicit and Python's zen is very right about it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#issuecomment-708449237
[The Workflow run](https://github.com/apache/airflow/actions/runs/306474344) is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#issuecomment-708545607
[The Workflow run](https://github.com/apache/airflow/actions/runs/306858335) is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#issuecomment-724237807
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.)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11526: Adds automated installation of dependent packages
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11526:
URL: https://github.com/apache/airflow/pull/11526#issuecomment-724192717
[The Workflow run](https://github.com/apache/airflow/actions/runs/354495172) 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*.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org