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/02/22 17:05:19 UTC
[GitHub] [airflow] potiuk opened a new pull request #7506: [AIRFLOW-6877]
check cross providers dependencies
potiuk opened a new pull request #7506: [AIRFLOW-6877] check cross providers dependencies
URL: https://github.com/apache/airflow/pull/7506
---
Issue link: WILL BE INSERTED BY [boring-cyborg](https://github.com/kaxil/boring-cyborg)
Make sure to mark the boxes below before creating PR: [x]
- [x] Description above provides context of the change
- [x] Commit message/PR title starts with `[AIRFLOW-NNNN]`. AIRFLOW-NNNN = JIRA ID<sup>*</sup>
- [x] Unit tests coverage for changes (not needed for documentation changes)
- [x] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
- [x] Relevant documentation is updated including usage instructions.
- [x] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
<sup>*</sup> For document-only changes commit message can start with `[AIRFLOW-XXXX]`.
---
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).
Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #7506:
[AIRFLOW-6877] [WIP] check cross providers dependencies depends on
[AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#discussion_r382938156
##########
File path: airflow/providers/google/cloud/operators/sftp_to_gcs.py
##########
@@ -19,15 +19,23 @@
This module contains SFTP to Google Cloud Storage operator.
"""
import os
+import warnings
from tempfile import NamedTemporaryFile
from typing import Optional, Union
from airflow.exceptions import AirflowException
from airflow.models import BaseOperator
-from airflow.providers.google.cloud.hooks.gcs import GCSHook
-from airflow.providers.sftp.hooks.sftp import SFTPHook
from airflow.utils.decorators import apply_defaults
+try:
+ from airflow.providers.google.cloud.hooks.gcs import GCSHook
+except ImportError:
+ warnings.warn("The package providers-google-cloud must be installed to use the operator.",
Review comment:
Ah yeah, that was one of the latest additions - I know detect such mistakes automatically (i.e no self-provider try/except is allowed) :). One of the 63 errors to fix :)
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #7506:
[AIRFLOW-6877] [WIP] check cross providers dependencies depends on
[AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#discussion_r383425146
##########
File path: airflow/providers/google/marketing_platform/operators/campaign_manager.py
##########
@@ -20,16 +20,23 @@
"""
import tempfile
import uuid
+import warnings
from typing import Any, Dict, List, Optional
from googleapiclient import http
from airflow.exceptions import AirflowException
from airflow.models import BaseOperator
-from airflow.providers.google.cloud.hooks.gcs import GCSHook
from airflow.providers.google.marketing_platform.hooks.campaign_manager import GoogleCampaignManagerHook
from airflow.utils.decorators import apply_defaults
+try:
+ from airflow.providers.google.cloud.hooks.gcs import GCSHook
+except ImportError:
+ warnings.warn("The package google-cloud must be installed to use the operator.",
Review comment:
> So actually.. now I want to change my "vote" to +1 for 2. And make sure that we define it like this "The first level is the provider that can be installed via a provider package which can contain sub-domains".
My thought was that it would be much better to have "operators"/"hooks"/"sensors" always staring as the "first" level of the provider. It think it makes much more sense to have all "operators" of the provider in one directory rather than split. But I think we are talking about the same, just looking at it from a different direction.
For me it should always be:
providers."PROVIDER".operators , where "PROVIDER" might have "." as well.
e.g.
providers.__apache.hive__.operators -> "apache.hive" being provider.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] feluelle commented on a change in pull request #7506:
[AIRFLOW-6877] [WIP] check cross providers dependencies depends on
[AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
feluelle commented on a change in pull request #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#discussion_r383403405
##########
File path: airflow/providers/google/marketing_platform/operators/campaign_manager.py
##########
@@ -20,16 +20,23 @@
"""
import tempfile
import uuid
+import warnings
from typing import Any, Dict, List, Optional
from googleapiclient import http
from airflow.exceptions import AirflowException
from airflow.models import BaseOperator
-from airflow.providers.google.cloud.hooks.gcs import GCSHook
from airflow.providers.google.marketing_platform.hooks.campaign_manager import GoogleCampaignManagerHook
from airflow.utils.decorators import apply_defaults
+try:
+ from airflow.providers.google.cloud.hooks.gcs import GCSHook
+except ImportError:
+ warnings.warn("The package google-cloud must be installed to use the operator.",
Review comment:
+1 for 1
Which would mean `cncf.kubernets` becomes `cncf`, `amazon.aws` becomes `amazon`, ...
Should the name we remove from the packages be added to the name of the hook/operator/sensor ? `aws_athena`, `aws_batch`, `aws_cloud_formation`, ... ?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #7506:
[AIRFLOW-6877] [WIP] check cross providers dependencies depends on
[AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#discussion_r383425146
##########
File path: airflow/providers/google/marketing_platform/operators/campaign_manager.py
##########
@@ -20,16 +20,23 @@
"""
import tempfile
import uuid
+import warnings
from typing import Any, Dict, List, Optional
from googleapiclient import http
from airflow.exceptions import AirflowException
from airflow.models import BaseOperator
-from airflow.providers.google.cloud.hooks.gcs import GCSHook
from airflow.providers.google.marketing_platform.hooks.campaign_manager import GoogleCampaignManagerHook
from airflow.utils.decorators import apply_defaults
+try:
+ from airflow.providers.google.cloud.hooks.gcs import GCSHook
+except ImportError:
+ warnings.warn("The package google-cloud must be installed to use the operator.",
Review comment:
> So actually.. now I want to change my "vote" to +1 for 2. And make sure that we define it like this "The first level is the provider that can be installed via a provider package which can contain sub-domains".
My thought was that it would be much better to have "operators"/"hooks"/"sensors" always staring as the "first" level of the provider. It think it makes much more sense to have all "operators" of the provider in one directory rather than split. But I think we are talking about the same, just looking at it from a different direction.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] feluelle commented on a change in pull request #7506:
[AIRFLOW-6877] [WIP] check cross providers dependencies depends on
[AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
feluelle commented on a change in pull request #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#discussion_r383416410
##########
File path: airflow/providers/google/marketing_platform/operators/campaign_manager.py
##########
@@ -20,16 +20,23 @@
"""
import tempfile
import uuid
+import warnings
from typing import Any, Dict, List, Optional
from googleapiclient import http
from airflow.exceptions import AirflowException
from airflow.models import BaseOperator
-from airflow.providers.google.cloud.hooks.gcs import GCSHook
from airflow.providers.google.marketing_platform.hooks.campaign_manager import GoogleCampaignManagerHook
from airflow.utils.decorators import apply_defaults
+try:
+ from airflow.providers.google.cloud.hooks.gcs import GCSHook
+except ImportError:
+ warnings.warn("The package google-cloud must be installed to use the operator.",
Review comment:
In my opinion the first level in the `providers` package is the provider. All the packages within are just names used in the context of the provider.
I would suggest to only consider the first package as a provider that can be installed via a seperate provider package.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #7506:
[AIRFLOW-6877] [WIP] check cross providers dependencies depends on
[AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#discussion_r383025733
##########
File path: airflow/providers/google/marketing_platform/operators/campaign_manager.py
##########
@@ -20,16 +20,23 @@
"""
import tempfile
import uuid
+import warnings
from typing import Any, Dict, List, Optional
from googleapiclient import http
from airflow.exceptions import AirflowException
from airflow.models import BaseOperator
-from airflow.providers.google.cloud.hooks.gcs import GCSHook
from airflow.providers.google.marketing_platform.hooks.campaign_manager import GoogleCampaignManagerHook
from airflow.utils.decorators import apply_defaults
+try:
+ from airflow.providers.google.cloud.hooks.gcs import GCSHook
+except ImportError:
+ warnings.warn("The package google-cloud must be installed to use the operator.",
Review comment:
I agree - but either we decide to move it all to one folder/provider or we leave it as three dependent packages. I do not want to make an exception to the structure of provider packages. It makes it much easier to have one approach where provider package (e.g. google.cloud) is under "providers/google/cloud/" directory and "operators/hooks/sensors" are right directly under it.
This way we do not have to keep any mapping between folders and provider packages - it is straightforward and can be easily derived from where the operators/hooks/sensors folders are (in fact I am already doing it when I check for import-cross-dependencies).
I am ok with both - separate packages and dependencies between them and moving everything back to "google" provider. Alternatively we could have google/common but I think it's the worst idea of the three. I have slight preference for option 1) but 2) is still OK.
Whatever we do - I think we should keep consistency between folder structure and "provider packages" structure (provider name is formed from the directories between "providers" and "operators" basically).
What do you think @mik-laj @nuclearpinguin @michalslowikowski00 @ANiteckiP @kaxil @ashb ?
Option 1):
1 single "google" provider package:
```
providers/google/
operators
hooks
sensors
```
Option 2)
3 separate providers: "google.cloud", "google.marketing", "google.suite". Both "gogle.marketing" and "google.suite" is pretty much useless without "google.cloud" (but it will be printed as warning when there is no google.cloud already installed..
```
providers/google/cloud
operators
...
providers/google/marketing
operators
...
providers/google/suite
operators
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk edited a comment on issue #7506: [AIRFLOW-6877]
[WIP] check cross providers dependencies depends on [AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#issuecomment-590098654
BTW, If you look at https://travis-ci.org/apache/airflow/jobs/653861799?utm_medium=notification&utm_source=github_status - you will se the kind of static check errors that are printed when one provider package imports directly another provider without try/except clause. There are just 63 cases left and I am happy to fix them all once we agree that this is good idea and right approach of handling cross-provider-package dependencies.
For example:
```
airflow/providers/amazon/aws/operators/gcs_to_s3.py. Provider: amazon.aws.
Import airflow.providers.google.cloud.operators.gcs.GCSListObjectsOperator,
provider: google.cloud. Fix it by adding try/except ImportError
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] nuclearpinguin commented on a change in pull request
#7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on
[AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
nuclearpinguin commented on a change in pull request #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#discussion_r382987065
##########
File path: airflow/providers/google/marketing_platform/operators/campaign_manager.py
##########
@@ -20,16 +20,23 @@
"""
import tempfile
import uuid
+import warnings
from typing import Any, Dict, List, Optional
from googleapiclient import http
from airflow.exceptions import AirflowException
from airflow.models import BaseOperator
-from airflow.providers.google.cloud.hooks.gcs import GCSHook
from airflow.providers.google.marketing_platform.hooks.campaign_manager import GoogleCampaignManagerHook
from airflow.utils.decorators import apply_defaults
+try:
+ from airflow.providers.google.cloud.hooks.gcs import GCSHook
+except ImportError:
+ warnings.warn("The package google-cloud must be installed to use the operator.",
Review comment:
In case of gmp we heavily use GooglsCloudBaseHook for hooks
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #7506:
[AIRFLOW-6877] [WIP] check cross providers dependencies depends on
[AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#discussion_r383425146
##########
File path: airflow/providers/google/marketing_platform/operators/campaign_manager.py
##########
@@ -20,16 +20,23 @@
"""
import tempfile
import uuid
+import warnings
from typing import Any, Dict, List, Optional
from googleapiclient import http
from airflow.exceptions import AirflowException
from airflow.models import BaseOperator
-from airflow.providers.google.cloud.hooks.gcs import GCSHook
from airflow.providers.google.marketing_platform.hooks.campaign_manager import GoogleCampaignManagerHook
from airflow.utils.decorators import apply_defaults
+try:
+ from airflow.providers.google.cloud.hooks.gcs import GCSHook
+except ImportError:
+ warnings.warn("The package google-cloud must be installed to use the operator.",
Review comment:
> So actually.. now I want to change my "vote" to +1 for 2. And make sure that we define it like this "The first level is the provider that can be installed via a provider package which can contain sub-domains".
My thought was that it would be much better to have "operators"/"hooks"/"sensors" always staring as the "first" level of the provider. It think it makes much more sense to have all "operators" of the provider in one directory rather than split. But I think we are talking about the same, just looking at it from a different direction.
For me it should always be:
providers."PROVIDER".operators , where "PROVIDER" might have "." as well.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #7506:
[AIRFLOW-6877] [WIP] check cross providers dependencies depends on
[AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#discussion_r383419743
##########
File path: airflow/providers/google/marketing_platform/operators/campaign_manager.py
##########
@@ -20,16 +20,23 @@
"""
import tempfile
import uuid
+import warnings
from typing import Any, Dict, List, Optional
from googleapiclient import http
from airflow.exceptions import AirflowException
from airflow.models import BaseOperator
-from airflow.providers.google.cloud.hooks.gcs import GCSHook
from airflow.providers.google.marketing_platform.hooks.campaign_manager import GoogleCampaignManagerHook
from airflow.utils.decorators import apply_defaults
+try:
+ from airflow.providers.google.cloud.hooks.gcs import GCSHook
+except ImportError:
+ warnings.warn("The package google-cloud must be installed to use the operator.",
Review comment:
> Which would mean `cncf.kubernets` becomes `cncf`, `amazon.aws` becomes `amazon`, ...
Not necessarily - I think we can still keep some sub-packages for those packages that have little cross dependencies. For example there are nearly no cross dependencies in "providers/apache" ...
The above discussion is strictly about "google" top-level provider and whether to split it or combine.I think it could be a decision made individually for each "top-level" provider - whether to split further or keep everything at one level. I think that's perfectly fine to have both options available.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #7506:
[AIRFLOW-6877] [WIP] check cross providers dependencies depends on
[AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#discussion_r383420796
##########
File path: airflow/providers/google/marketing_platform/operators/campaign_manager.py
##########
@@ -20,16 +20,23 @@
"""
import tempfile
import uuid
+import warnings
from typing import Any, Dict, List, Optional
from googleapiclient import http
from airflow.exceptions import AirflowException
from airflow.models import BaseOperator
-from airflow.providers.google.cloud.hooks.gcs import GCSHook
from airflow.providers.google.marketing_platform.hooks.campaign_manager import GoogleCampaignManagerHook
from airflow.utils.decorators import apply_defaults
+try:
+ from airflow.providers.google.cloud.hooks.gcs import GCSHook
+except ImportError:
+ warnings.warn("The package google-cloud must be installed to use the operator.",
Review comment:
BTW - this is still following what we already agreed to in AIP-21 - the rule about whether to split or not was left up to the top-level provider.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk edited a comment on issue #7506: [AIRFLOW-6877]
[WIP] check cross providers dependencies depends on [AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#issuecomment-590098654
BTW, If you look at https://travis-ci.org/apache/airflow/jobs/653861799?utm_medium=notification&utm_source=github_status - you will se the kind of static check errors that are printed when one provider package imports directly another provider without try/except clause. There are just 63 cases left and I am happy to fix them all once we agree that this is good idea and right approach of handling cross-provider-package dependencies.
For example:
```
airflow/providers/amazon/aws/operators/gcs_to_s3.py. Provider: amazon.aws. Import airflow.providers.google.cloud.operators.gcs.GCSListObjectsOperator, provider: google.cloud. Fix it by adding try/except ImportError
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] nuclearpinguin commented on a change in pull request
#7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on
[AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
nuclearpinguin commented on a change in pull request #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#discussion_r382935850
##########
File path: airflow/providers/google/marketing_platform/operators/campaign_manager.py
##########
@@ -20,16 +20,23 @@
"""
import tempfile
import uuid
+import warnings
from typing import Any, Dict, List, Optional
from googleapiclient import http
from airflow.exceptions import AirflowException
from airflow.models import BaseOperator
-from airflow.providers.google.cloud.hooks.gcs import GCSHook
from airflow.providers.google.marketing_platform.hooks.campaign_manager import GoogleCampaignManagerHook
from airflow.utils.decorators import apply_defaults
+try:
+ from airflow.providers.google.cloud.hooks.gcs import GCSHook
+except ImportError:
+ warnings.warn("The package google-cloud must be installed to use the operator.",
Review comment:
In this case I think we should bundle gcp and gmp as all "report" operators will require GCS or BQ hooks.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on issue #7506: [AIRFLOW-6877] Add
cross-provider dependencies as extras
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #7506: [AIRFLOW-6877] Add cross-provider dependencies as extras
URL: https://github.com/apache/airflow/pull/7506#issuecomment-594222622
This one should be I think mergeable. We have cross-dependencies added to backport packages as extras instead of the warnings. I think it makes much more sense and is much lighter
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #7506:
[AIRFLOW-6877] [WIP] check cross providers dependencies depends on
[AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#discussion_r383025783
##########
File path: airflow/providers/google/marketing_platform/operators/campaign_manager.py
##########
@@ -20,16 +20,23 @@
"""
import tempfile
import uuid
+import warnings
from typing import Any, Dict, List, Optional
from googleapiclient import http
from airflow.exceptions import AirflowException
from airflow.models import BaseOperator
-from airflow.providers.google.cloud.hooks.gcs import GCSHook
from airflow.providers.google.marketing_platform.hooks.campaign_manager import GoogleCampaignManagerHook
from airflow.utils.decorators import apply_defaults
+try:
+ from airflow.providers.google.cloud.hooks.gcs import GCSHook
+except ImportError:
+ warnings.warn("The package google-cloud must be installed to use the operator.",
Review comment:
1️⃣ is my prefered choice
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] nuclearpinguin commented on a change in pull request
#7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on
[AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
nuclearpinguin commented on a change in pull request #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#discussion_r383169057
##########
File path: airflow/providers/google/marketing_platform/operators/campaign_manager.py
##########
@@ -20,16 +20,23 @@
"""
import tempfile
import uuid
+import warnings
from typing import Any, Dict, List, Optional
from googleapiclient import http
from airflow.exceptions import AirflowException
from airflow.models import BaseOperator
-from airflow.providers.google.cloud.hooks.gcs import GCSHook
from airflow.providers.google.marketing_platform.hooks.campaign_manager import GoogleCampaignManagerHook
from airflow.utils.decorators import apply_defaults
+try:
+ from airflow.providers.google.cloud.hooks.gcs import GCSHook
+except ImportError:
+ warnings.warn("The package google-cloud must be installed to use the operator.",
Review comment:
+1 for 1, in this way users don't have to care under which Google umbrella service is
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on issue #7506: [AIRFLOW-6877] [WIP]
check cross providers dependencies depends on [AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#issuecomment-593271593
@feluelle @mik-laj @nuclearpinguin -> I thought a bit about this and I think we can actually already use the mapping that we have in setup.py of the package configuration. We need to have it anyway to keep package -> dependencies mapping. Since we already have it, there will be no need to maintain any extra mapping and I think we should be perfectly fine even if we leave the internal structure in "google". I will update and rebase the PR shortly (today likely) and I hope we will be able to proceed to voting.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #7506:
[AIRFLOW-6877] [WIP] check cross providers dependencies depends on
[AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#discussion_r383026128
##########
File path: airflow/providers/google/marketing_platform/operators/campaign_manager.py
##########
@@ -20,16 +20,23 @@
"""
import tempfile
import uuid
+import warnings
from typing import Any, Dict, List, Optional
from googleapiclient import http
from airflow.exceptions import AirflowException
from airflow.models import BaseOperator
-from airflow.providers.google.cloud.hooks.gcs import GCSHook
from airflow.providers.google.marketing_platform.hooks.campaign_manager import GoogleCampaignManagerHook
from airflow.utils.decorators import apply_defaults
+try:
+ from airflow.providers.google.cloud.hooks.gcs import GCSHook
+except ImportError:
+ warnings.warn("The package google-cloud must be installed to use the operator.",
Review comment:
BTW, If you look at https://travis-ci.org/apache/airflow/jobs/653861799?utm_medium=notification&utm_source=github_status - you will se the kind of static check errors that are printed when one provider package imports directly another provider without try/except clause. There are just 63 cases left and I am happy to fix them all once we agree that this is good idea and right approach of handling cross-provider-package dependencies.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #7506:
[AIRFLOW-6877] [WIP] check cross providers dependencies depends on
[AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#discussion_r382937981
##########
File path: airflow/providers/google/marketing_platform/operators/campaign_manager.py
##########
@@ -20,16 +20,23 @@
"""
import tempfile
import uuid
+import warnings
from typing import Any, Dict, List, Optional
from googleapiclient import http
from airflow.exceptions import AirflowException
from airflow.models import BaseOperator
-from airflow.providers.google.cloud.hooks.gcs import GCSHook
from airflow.providers.google.marketing_platform.hooks.campaign_manager import GoogleCampaignManagerHook
from airflow.utils.decorators import apply_defaults
+try:
+ from airflow.providers.google.cloud.hooks.gcs import GCSHook
+except ImportError:
+ warnings.warn("The package google-cloud must be installed to use the operator.",
Review comment:
I think this is still OK as it is IMHO. I want to make it a good case to test multiple packages and I would not make an exception here. What we could do is to make intra-dependencies and automatically install google.cloud when gmp is used. We should make some consistent rules and stick to it - if we want to group some packages, we should group them. If we want to split - we should keep them separated.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #7506:
[AIRFLOW-6877] [WIP] check cross providers dependencies depends on
[AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#discussion_r383425146
##########
File path: airflow/providers/google/marketing_platform/operators/campaign_manager.py
##########
@@ -20,16 +20,23 @@
"""
import tempfile
import uuid
+import warnings
from typing import Any, Dict, List, Optional
from googleapiclient import http
from airflow.exceptions import AirflowException
from airflow.models import BaseOperator
-from airflow.providers.google.cloud.hooks.gcs import GCSHook
from airflow.providers.google.marketing_platform.hooks.campaign_manager import GoogleCampaignManagerHook
from airflow.utils.decorators import apply_defaults
+try:
+ from airflow.providers.google.cloud.hooks.gcs import GCSHook
+except ImportError:
+ warnings.warn("The package google-cloud must be installed to use the operator.",
Review comment:
> So actually.. now I want to change my "vote" to +1 for 2. And make sure that we define it like this "The first level is the provider that can be installed via a provider package which can contain sub-domains".
My thought was that it would be much better to have "operators"/"hooks"/"sensors" always staring as the "first" level of the provider. It think it makes much more sense to have all "operators" of the provider in one directory rather than split.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on issue #7506: [AIRFLOW-6877] Add
cross-provider dependencies as extras
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #7506: [AIRFLOW-6877] Add cross-provider dependencies as extras
URL: https://github.com/apache/airflow/pull/7506#issuecomment-596100427
all green @nuclearpinguin !
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] nuclearpinguin commented on issue #7506: [AIRFLOW-6877]
Add cross-provider dependencies as extras
Posted by GitBox <gi...@apache.org>.
nuclearpinguin commented on issue #7506: [AIRFLOW-6877] Add cross-provider dependencies as extras
URL: https://github.com/apache/airflow/pull/7506#issuecomment-596072390
@potiuk
```
You've added a test file currently listed as missing:
tests/providers/microsoft/mssql/operators/test_mssql.py
Thank you very much.
Can you remove it from the list of missing tests, please?
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] nuclearpinguin commented on a change in pull request
#7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on
[AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
nuclearpinguin commented on a change in pull request #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#discussion_r382935600
##########
File path: airflow/providers/amazon/aws/operators/s3_to_redshift.py
##########
@@ -15,13 +15,20 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+import warnings
from typing import List, Optional, Union
from airflow.models import BaseOperator
from airflow.providers.amazon.aws.hooks.s3 import S3Hook
-from airflow.providers.postgres.hooks.postgres import PostgresHook
from airflow.utils.decorators import apply_defaults
+try:
+ from airflow.providers.postgres.hooks.postgres import PostgresHook
+except ImportError:
+ warnings.warn("The package providers-amazon-aws must be installed to use the operator.",
Review comment:
I am not sure I understand this warning... should it be `postgres` instead of `provider-amazon-aws`?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] kaxil commented on a change in pull request #7506:
[AIRFLOW-6877] Add cross-provider dependencies as extras
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #7506: [AIRFLOW-6877] Add cross-provider dependencies as extras
URL: https://github.com/apache/airflow/pull/7506#discussion_r389267392
##########
File path: tests/build_provider_packages_dependencies.py
##########
@@ -0,0 +1,269 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import json
+import os
+import sys
+from ast import Import, ImportFrom, NodeVisitor, parse
+from collections import defaultdict
+from os.path import dirname, sep
+from typing import Dict, List, Optional
+
+from backport_packages.setup_backport_packages import PROVIDERS_DEPENDENCIES
+
+sys.path.append(os.path.join(dirname(__file__), os.pardir))
+
+
+AIRFLOW_PROVIDERS_FILE_PREFIX = "airflow" + sep + "providers" + sep
+AIRFLOW_TESTS_PROVIDERS_FILE_PREFIX = "tests" + sep + "providers" + sep
+AIRFLOW_PROVIDERS_IMPORT_PREFIX = "airflow.providers."
+
+# List of information messages generated
+infos: List[str] = []
+# List of warnings generated
+warnings: List[str] = []
+# list of errors generated
+errors: List[str] = []
+
+# store dependencies
+dependencies: Dict[str, List[str]] = defaultdict(list)
+
+
+def find_provider(provider_elements: List[str]) -> Optional[str]:
+ """
+ Finds provider name from the list of elements provided. It looks the providers up
+ in PROVIDERS_DEPENDENCIES map taken from the provider's package setup.
+
+ :param provider_elements: array of elements of the path (split)
+ :return: provider name or None if no provider could be found
+ """
+ provider = ""
+ separator = ""
+ provider_keys = PROVIDERS_DEPENDENCIES.keys()
+ for element in provider_elements:
+ provider = provider + separator + element
+ if provider in provider_keys:
+ return provider
+ separator = "."
+ return None
+
+
+def get_provider_from_file_name(file_name: str) -> Optional[str]:
+ """
+ Retrieves provider name from file name
+ :param file_name: name of the file
+ :return: provider name or None if no provider could be found
+ """
+ if AIRFLOW_PROVIDERS_FILE_PREFIX not in file_name and \
+ AIRFLOW_TESTS_PROVIDERS_FILE_PREFIX not in file_name:
+ # We should only check file that are provider
+ errors.append(f"Wrong file not in the providers package = {file_name}")
+ return None
+ suffix = get_file_suffix(file_name)
+ split_path = suffix.split(sep)[2:]
+ provider = find_provider(split_path)
+ if not provider and file_name.endswith("__init__.py"):
+ infos.append(f"Skipped file = {file_name}")
+ elif not provider:
+ warnings.append(f"Provider not found for path = {file_name}")
+ return provider
+
+
+def get_file_suffix(file_name):
+ if AIRFLOW_PROVIDERS_FILE_PREFIX in file_name:
+ return file_name[file_name.find(AIRFLOW_PROVIDERS_FILE_PREFIX):]
+ if AIRFLOW_TESTS_PROVIDERS_FILE_PREFIX in file_name:
+ return file_name[file_name.find(AIRFLOW_TESTS_PROVIDERS_FILE_PREFIX):]
+ return None
+
+
+def get_provider_from_import(import_name: str) -> Optional[str]:
+ """
+ Retrieves provider name from file name
+ :param import_name: name of the import
+ :return: provider name or None if no provider could be found
+ """
+ if AIRFLOW_PROVIDERS_IMPORT_PREFIX not in import_name:
+ # skip silently - we expect non-providers imports
+ return None
+ suffix = import_name[import_name.find(AIRFLOW_PROVIDERS_IMPORT_PREFIX):]
+ split_import = suffix.split(".")[2:]
+ provider = find_provider(split_import)
+ if not provider:
+ warnings.append(f"Provider not found for import = {import_name}")
+ return provider
+
+
+class ImportFinder(NodeVisitor):
+ """
+ AST visitor that collects all imported names in its imports
+ """
+ def __init__(self, filename):
+ self.imports: List[str] = []
+ self.filename = filename
+ self.handled_import_exception = List[str]
+ self.tried_imports: List[str] = []
+
+ def process_import(self, import_name: str):
+ self.imports.append(import_name)
+
+ # noinspection PyMethodMayBeStatic
+ def get_import_name_from_import_from(self, node: ImportFrom) -> List[str]:
+ """
+ Retrieves import name from the "from" import.
+ :param node: ImportFrom name
+ :return: import name
+ """
+ import_names: List[str] = []
+ for alias in node.names:
+ name = alias.name
+ fullname = '%s.%s' % (node.module, name) if node.module else name
+ import_names.append(fullname)
+ return import_names
+
+ # noinspection PyPep8Naming
+ def visit_Import(self, node: Import): # pylint: disable=invalid-name
+ for alias in node.names:
+ self.process_import(alias.name)
+
+ # noinspection PyPep8Naming
+ def visit_ImportFrom(self, node: ImportFrom): # pylint: disable=invalid-name
+ if node.module == '__future__':
+ return
+ for fullname in self.get_import_name_from_import_from(node):
+ self.process_import(fullname)
+
+
+def get_imports_from_file(file_name: str) -> List[str]:
+ """
+ Retrieves imports from file.
+ :param file_name: name of the file
+ :return: list of import names
+ """
+ with open(file_name) as f:
+ root = parse(f.read(), file_name)
+ visitor = ImportFinder(file_name)
+ visitor.visit(root)
+ return visitor.imports
+
+
+def check_if_different_provider_used(file_name: str):
+ file_provider = get_provider_from_file_name(file_name)
+ if not file_provider:
+ return
+ imports = get_imports_from_file(file_name)
+ for import_name in imports:
+ import_provider = get_provider_from_import(import_name)
+ if import_provider and file_provider != import_provider:
+ dependencies[file_provider].append(import_provider)
+
+
+def parse_arguments():
+ import argparse
+ parser = argparse.ArgumentParser(
+ description='Checks if dependencies between packages are handled correctly.')
+ parser.add_argument("-f", "--provider-dependencies-file",
+ help="Stores dependencies between providers in the file")
+ parser.add_argument("-d", "--documentation-file",
+ help="Updates package documentation in the file specified (.rst)")
+ parser.add_argument('files', nargs='*')
+ args = parser.parse_args()
+
+ if len(args.files) < 1:
+ parser.print_usage()
+ print()
+ sys.exit(2)
+ return args.files, args.provider_dependencies_file, args.documentation_file
+
+
+PREFIX = " "
+
+HEADER = """
+========================== ===========================
+Package Extras
+========================== ===========================
+"""
+FOOTER = """========================== ===========================
+
+"""
+
+
+def insert_documentation(deps_dict: Dict[str, List[str]], res: List[str]):
+ res += HEADER.splitlines(keepends=True)
+ for package, deps in deps_dict.items():
+ deps_str = ",".join(deps)
+ res.append(f"{package:27}{deps_str}\n")
+ res += FOOTER.splitlines(keepends=True)
+
+
+if __name__ == '__main__':
+ print()
Review comment:
Used it for debugging? We can remove 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
With regards,
Apache Git Services
[GitHub] [airflow] nuclearpinguin commented on a change in pull request
#7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on
[AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
nuclearpinguin commented on a change in pull request #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#discussion_r382935743
##########
File path: airflow/providers/google/cloud/operators/sftp_to_gcs.py
##########
@@ -19,15 +19,23 @@
This module contains SFTP to Google Cloud Storage operator.
"""
import os
+import warnings
from tempfile import NamedTemporaryFile
from typing import Optional, Union
from airflow.exceptions import AirflowException
from airflow.models import BaseOperator
-from airflow.providers.google.cloud.hooks.gcs import GCSHook
-from airflow.providers.sftp.hooks.sftp import SFTPHook
from airflow.utils.decorators import apply_defaults
+try:
+ from airflow.providers.google.cloud.hooks.gcs import GCSHook
+except ImportError:
+ warnings.warn("The package providers-google-cloud must be installed to use the operator.",
Review comment:
Should it be `providers-ssh` instead of gcp?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #7506:
[AIRFLOW-6877] [WIP] check cross providers dependencies depends on
[AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#discussion_r383233043
##########
File path: airflow/providers/google/marketing_platform/operators/campaign_manager.py
##########
@@ -20,16 +20,23 @@
"""
import tempfile
import uuid
+import warnings
from typing import Any, Dict, List, Optional
from googleapiclient import http
from airflow.exceptions import AirflowException
from airflow.models import BaseOperator
-from airflow.providers.google.cloud.hooks.gcs import GCSHook
from airflow.providers.google.marketing_platform.hooks.campaign_manager import GoogleCampaignManagerHook
from airflow.utils.decorators import apply_defaults
+try:
+ from airflow.providers.google.cloud.hooks.gcs import GCSHook
+except ImportError:
+ warnings.warn("The package google-cloud must be installed to use the operator.",
Review comment:
@mik-laj @ANiteckiP @michalslowikowski00 @kaxil ?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk merged pull request #7506: [AIRFLOW-6877] Add
cross-provider dependencies as extras
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #7506: [AIRFLOW-6877] Add cross-provider dependencies as extras
URL: https://github.com/apache/airflow/pull/7506
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] feluelle commented on a change in pull request #7506:
[AIRFLOW-6877] [WIP] check cross providers dependencies depends on
[AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
feluelle commented on a change in pull request #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#discussion_r383416410
##########
File path: airflow/providers/google/marketing_platform/operators/campaign_manager.py
##########
@@ -20,16 +20,23 @@
"""
import tempfile
import uuid
+import warnings
from typing import Any, Dict, List, Optional
from googleapiclient import http
from airflow.exceptions import AirflowException
from airflow.models import BaseOperator
-from airflow.providers.google.cloud.hooks.gcs import GCSHook
from airflow.providers.google.marketing_platform.hooks.campaign_manager import GoogleCampaignManagerHook
from airflow.utils.decorators import apply_defaults
+try:
+ from airflow.providers.google.cloud.hooks.gcs import GCSHook
+except ImportError:
+ warnings.warn("The package google-cloud must be installed to use the operator.",
Review comment:
In my opinion the first level in the `providers` package is the provider. All the packages within are just names used in the context of the provider.
I would suggest to only consider the first package as a provider that can be installed via a separate provider package.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on issue #7506: [AIRFLOW-6877] Add
cross-provider dependencies as extras
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #7506: [AIRFLOW-6877] Add cross-provider dependencies as extras
URL: https://github.com/apache/airflow/pull/7506#issuecomment-596093663
Fixed @nuclearpinguin ! Thanks!
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #7506:
[AIRFLOW-6877] [WIP] check cross providers dependencies depends on
[AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#discussion_r382938276
##########
File path: airflow/providers/amazon/aws/operators/s3_to_redshift.py
##########
@@ -15,13 +15,20 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+import warnings
from typing import List, Optional, Union
from airflow.models import BaseOperator
from airflow.providers.amazon.aws.hooks.s3 import S3Hook
-from airflow.providers.postgres.hooks.postgres import PostgresHook
from airflow.utils.decorators import apply_defaults
+try:
+ from airflow.providers.postgres.hooks.postgres import PostgresHook
+except ImportError:
+ warnings.warn("The package providers-amazon-aws must be installed to use the operator.",
Review comment:
Yeah, It was manually copy-pasted and it's yet something to be reviewed. ... I have not fixed all of these and I want to change it to be more difficult to make copy-paste mistake
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #7506:
[AIRFLOW-6877] [WIP] check cross providers dependencies depends on
[AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#discussion_r383026128
##########
File path: airflow/providers/google/marketing_platform/operators/campaign_manager.py
##########
@@ -20,16 +20,23 @@
"""
import tempfile
import uuid
+import warnings
from typing import Any, Dict, List, Optional
from googleapiclient import http
from airflow.exceptions import AirflowException
from airflow.models import BaseOperator
-from airflow.providers.google.cloud.hooks.gcs import GCSHook
from airflow.providers.google.marketing_platform.hooks.campaign_manager import GoogleCampaignManagerHook
from airflow.utils.decorators import apply_defaults
+try:
+ from airflow.providers.google.cloud.hooks.gcs import GCSHook
+except ImportError:
+ warnings.warn("The package google-cloud must be installed to use the operator.",
Review comment:
BTW, If you look at https://travis-ci.org/apache/airflow/jobs/653861799?utm_medium=notification&utm_source=github_status - you will se the kind of static check errors that are printed when one provider package imports directly another provider without try/except clause. There are just 63 cases left and I am happy to fix them all once we agree that this is good idea and right approach of handling cross-provider-package dependencies.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] feluelle commented on a change in pull request #7506:
[AIRFLOW-6877] [WIP] check cross providers dependencies depends on
[AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
feluelle commented on a change in pull request #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#discussion_r383418975
##########
File path: airflow/providers/google/marketing_platform/operators/campaign_manager.py
##########
@@ -20,16 +20,23 @@
"""
import tempfile
import uuid
+import warnings
from typing import Any, Dict, List, Optional
from googleapiclient import http
from airflow.exceptions import AirflowException
from airflow.models import BaseOperator
-from airflow.providers.google.cloud.hooks.gcs import GCSHook
from airflow.providers.google.marketing_platform.hooks.campaign_manager import GoogleCampaignManagerHook
from airflow.utils.decorators import apply_defaults
+try:
+ from airflow.providers.google.cloud.hooks.gcs import GCSHook
+except ImportError:
+ warnings.warn("The package google-cloud must be installed to use the operator.",
Review comment:
So actually.. now I want to change my "vote" to +1 for 2. And make sure that we define it like this "The first level is the provider that can be installed via a provider package which can contain sub-domains".
WDYT?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #7506:
[AIRFLOW-6877] Add cross-provider dependencies as extras
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #7506: [AIRFLOW-6877] Add cross-provider dependencies as extras
URL: https://github.com/apache/airflow/pull/7506#discussion_r389295874
##########
File path: tests/build_provider_packages_dependencies.py
##########
@@ -0,0 +1,269 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import json
+import os
+import sys
+from ast import Import, ImportFrom, NodeVisitor, parse
+from collections import defaultdict
+from os.path import dirname, sep
+from typing import Dict, List, Optional
+
+from backport_packages.setup_backport_packages import PROVIDERS_DEPENDENCIES
+
+sys.path.append(os.path.join(dirname(__file__), os.pardir))
+
+
+AIRFLOW_PROVIDERS_FILE_PREFIX = "airflow" + sep + "providers" + sep
+AIRFLOW_TESTS_PROVIDERS_FILE_PREFIX = "tests" + sep + "providers" + sep
+AIRFLOW_PROVIDERS_IMPORT_PREFIX = "airflow.providers."
+
+# List of information messages generated
+infos: List[str] = []
+# List of warnings generated
+warnings: List[str] = []
+# list of errors generated
+errors: List[str] = []
+
+# store dependencies
+dependencies: Dict[str, List[str]] = defaultdict(list)
+
+
+def find_provider(provider_elements: List[str]) -> Optional[str]:
+ """
+ Finds provider name from the list of elements provided. It looks the providers up
+ in PROVIDERS_DEPENDENCIES map taken from the provider's package setup.
+
+ :param provider_elements: array of elements of the path (split)
+ :return: provider name or None if no provider could be found
+ """
+ provider = ""
+ separator = ""
+ provider_keys = PROVIDERS_DEPENDENCIES.keys()
+ for element in provider_elements:
+ provider = provider + separator + element
+ if provider in provider_keys:
+ return provider
+ separator = "."
+ return None
+
+
+def get_provider_from_file_name(file_name: str) -> Optional[str]:
+ """
+ Retrieves provider name from file name
+ :param file_name: name of the file
+ :return: provider name or None if no provider could be found
+ """
+ if AIRFLOW_PROVIDERS_FILE_PREFIX not in file_name and \
+ AIRFLOW_TESTS_PROVIDERS_FILE_PREFIX not in file_name:
+ # We should only check file that are provider
+ errors.append(f"Wrong file not in the providers package = {file_name}")
+ return None
+ suffix = get_file_suffix(file_name)
+ split_path = suffix.split(sep)[2:]
+ provider = find_provider(split_path)
+ if not provider and file_name.endswith("__init__.py"):
+ infos.append(f"Skipped file = {file_name}")
+ elif not provider:
+ warnings.append(f"Provider not found for path = {file_name}")
+ return provider
+
+
+def get_file_suffix(file_name):
+ if AIRFLOW_PROVIDERS_FILE_PREFIX in file_name:
+ return file_name[file_name.find(AIRFLOW_PROVIDERS_FILE_PREFIX):]
+ if AIRFLOW_TESTS_PROVIDERS_FILE_PREFIX in file_name:
+ return file_name[file_name.find(AIRFLOW_TESTS_PROVIDERS_FILE_PREFIX):]
+ return None
+
+
+def get_provider_from_import(import_name: str) -> Optional[str]:
+ """
+ Retrieves provider name from file name
+ :param import_name: name of the import
+ :return: provider name or None if no provider could be found
+ """
+ if AIRFLOW_PROVIDERS_IMPORT_PREFIX not in import_name:
+ # skip silently - we expect non-providers imports
+ return None
+ suffix = import_name[import_name.find(AIRFLOW_PROVIDERS_IMPORT_PREFIX):]
+ split_import = suffix.split(".")[2:]
+ provider = find_provider(split_import)
+ if not provider:
+ warnings.append(f"Provider not found for import = {import_name}")
+ return provider
+
+
+class ImportFinder(NodeVisitor):
+ """
+ AST visitor that collects all imported names in its imports
+ """
+ def __init__(self, filename):
+ self.imports: List[str] = []
+ self.filename = filename
+ self.handled_import_exception = List[str]
+ self.tried_imports: List[str] = []
+
+ def process_import(self, import_name: str):
+ self.imports.append(import_name)
+
+ # noinspection PyMethodMayBeStatic
+ def get_import_name_from_import_from(self, node: ImportFrom) -> List[str]:
+ """
+ Retrieves import name from the "from" import.
+ :param node: ImportFrom name
+ :return: import name
+ """
+ import_names: List[str] = []
+ for alias in node.names:
+ name = alias.name
+ fullname = '%s.%s' % (node.module, name) if node.module else name
+ import_names.append(fullname)
+ return import_names
+
+ # noinspection PyPep8Naming
+ def visit_Import(self, node: Import): # pylint: disable=invalid-name
+ for alias in node.names:
+ self.process_import(alias.name)
+
+ # noinspection PyPep8Naming
+ def visit_ImportFrom(self, node: ImportFrom): # pylint: disable=invalid-name
+ if node.module == '__future__':
+ return
+ for fullname in self.get_import_name_from_import_from(node):
+ self.process_import(fullname)
+
+
+def get_imports_from_file(file_name: str) -> List[str]:
+ """
+ Retrieves imports from file.
+ :param file_name: name of the file
+ :return: list of import names
+ """
+ with open(file_name) as f:
+ root = parse(f.read(), file_name)
+ visitor = ImportFinder(file_name)
+ visitor.visit(root)
+ return visitor.imports
+
+
+def check_if_different_provider_used(file_name: str):
+ file_provider = get_provider_from_file_name(file_name)
+ if not file_provider:
+ return
+ imports = get_imports_from_file(file_name)
+ for import_name in imports:
+ import_provider = get_provider_from_import(import_name)
+ if import_provider and file_provider != import_provider:
+ dependencies[file_provider].append(import_provider)
+
+
+def parse_arguments():
+ import argparse
+ parser = argparse.ArgumentParser(
+ description='Checks if dependencies between packages are handled correctly.')
+ parser.add_argument("-f", "--provider-dependencies-file",
+ help="Stores dependencies between providers in the file")
+ parser.add_argument("-d", "--documentation-file",
+ help="Updates package documentation in the file specified (.rst)")
+ parser.add_argument('files', nargs='*')
+ args = parser.parse_args()
+
+ if len(args.files) < 1:
+ parser.print_usage()
+ print()
+ sys.exit(2)
+ return args.files, args.provider_dependencies_file, args.documentation_file
+
+
+PREFIX = " "
+
+HEADER = """
+========================== ===========================
+Package Extras
+========================== ===========================
+"""
+FOOTER = """========================== ===========================
+
+"""
+
+
+def insert_documentation(deps_dict: Dict[str, List[str]], res: List[str]):
+ res += HEADER.splitlines(keepends=True)
+ for package, deps in deps_dict.items():
+ deps_str = ",".join(deps)
+ res.append(f"{package:27}{deps_str}\n")
+ res += FOOTER.splitlines(keepends=True)
+
+
+if __name__ == '__main__':
+ print()
Review comment:
No it's actually used to generate list of dependencies and write them to documentation in pre-commit :).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on issue #7506: [AIRFLOW-6877] [WIP]
check cross providers dependencies depends on [AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#issuecomment-590098654
BTW, If you look at https://travis-ci.org/apache/airflow/jobs/653861799?utm_medium=notification&utm_source=github_status - you will se the kind of static check errors that are printed when one provider package imports directly another provider without try/except clause. There are just 63 cases left and I am happy to fix them all once we agree that this is good idea and right approach of handling cross-provider-package dependencies.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] feluelle commented on a change in pull request #7506:
[AIRFLOW-6877] [WIP] check cross providers dependencies depends on
[AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
feluelle commented on a change in pull request #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#discussion_r383389611
##########
File path: README.md
##########
@@ -132,6 +132,9 @@ environments to Airflow 2.0.
More information about the status and releases of the back-ported packages are available
at [Backported providers package page](https://cwiki.apache.org/confluence/display/AIRFLOW/Backported+providers+packages+for+Airflow+1.10.*+series)
+Dependencies between packages are stored in ``airflow/providers/dependencies.json``. See
+[CONTRIB.rst]()
Review comment:
```suggestion
[CONTRIBUTING.rst](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst)
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io commented on issue #7506: [AIRFLOW-6877] Add
cross-provider dependencies as extras
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #7506: [AIRFLOW-6877] Add cross-provider dependencies as extras
URL: https://github.com/apache/airflow/pull/7506#issuecomment-596100131
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7506?src=pr&el=h1) Report
> Merging [#7506](https://codecov.io/gh/apache/airflow/pull/7506?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/f3abd340826289dec23e96b79a6ed9b6a1955027?src=pr&el=desc) will **increase** coverage by `0.01%`.
> The diff coverage is `100%`.
[](https://codecov.io/gh/apache/airflow/pull/7506?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7506 +/- ##
==========================================
+ Coverage 86.83% 86.84% +0.01%
==========================================
Files 897 897
Lines 42805 42804 -1
==========================================
+ Hits 37170 37174 +4
+ Misses 5635 5630 -5
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7506?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/providers/sftp/operators/sftp.py](https://codecov.io/gh/apache/airflow/pull/7506/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvc2Z0cC9vcGVyYXRvcnMvc2Z0cC5weQ==) | `87.67% <ø> (ø)` | :arrow_up: |
| [...rflow/providers/amazon/aws/operators/sftp\_to\_s3.py](https://codecov.io/gh/apache/airflow/pull/7506/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYW1hem9uL2F3cy9vcGVyYXRvcnMvc2Z0cF90b19zMy5weQ==) | `100% <ø> (ø)` | :arrow_up: |
| [.../cloud/operators/cloud\_storage\_transfer\_service.py](https://codecov.io/gh/apache/airflow/pull/7506/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL29wZXJhdG9ycy9jbG91ZF9zdG9yYWdlX3RyYW5zZmVyX3NlcnZpY2UucHk=) | `95.62% <100%> (-0.02%)` | :arrow_down: |
| [...cncf/kubernetes/example\_dags/example\_kubernetes.py](https://codecov.io/gh/apache/airflow/pull/7506/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvY25jZi9rdWJlcm5ldGVzL2V4YW1wbGVfZGFncy9leGFtcGxlX2t1YmVybmV0ZXMucHk=) | `100% <100%> (+21.42%)` | :arrow_up: |
| [airflow/jobs/scheduler\_job.py](https://codecov.io/gh/apache/airflow/pull/7506/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL3NjaGVkdWxlcl9qb2IucHk=) | `90.94% <0%> (+0.29%)` | :arrow_up: |
| [airflow/utils/sqlalchemy.py](https://codecov.io/gh/apache/airflow/pull/7506/diff?src=pr&el=tree#diff-YWlyZmxvdy91dGlscy9zcWxhbGNoZW15LnB5) | `85.71% <0%> (+0.78%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7506?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7506?src=pr&el=footer). Last update [f3abd34...6a7b5f9](https://codecov.io/gh/apache/airflow/pull/7506?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #7506:
[AIRFLOW-6877] [WIP] check cross providers dependencies depends on
[AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#discussion_r387087889
##########
File path: airflow/providers/google/marketing_platform/operators/campaign_manager.py
##########
@@ -20,16 +20,23 @@
"""
import tempfile
import uuid
+import warnings
from typing import Any, Dict, List, Optional
from googleapiclient import http
from airflow.exceptions import AirflowException
from airflow.models import BaseOperator
-from airflow.providers.google.cloud.hooks.gcs import GCSHook
from airflow.providers.google.marketing_platform.hooks.campaign_manager import GoogleCampaignManagerHook
from airflow.utils.decorators import apply_defaults
+try:
+ from airflow.providers.google.cloud.hooks.gcs import GCSHook
+except ImportError:
+ warnings.warn("The package google-cloud must be installed to use the operator.",
Review comment:
OK. I found an easy way - i can use the hashmap from the setup_backport_opertors to define what are the packages. Without adding any extra mapping. This way we can still keep the "google" related packages in a single "backport package" but with internal "cloud/marketing/suite" structure. Updated PR shortly and I will make all the try/except corrections and rebase to make it ready for review.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #7506:
[AIRFLOW-6877] [WIP] check cross providers dependencies depends on
[AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#discussion_r383025733
##########
File path: airflow/providers/google/marketing_platform/operators/campaign_manager.py
##########
@@ -20,16 +20,23 @@
"""
import tempfile
import uuid
+import warnings
from typing import Any, Dict, List, Optional
from googleapiclient import http
from airflow.exceptions import AirflowException
from airflow.models import BaseOperator
-from airflow.providers.google.cloud.hooks.gcs import GCSHook
from airflow.providers.google.marketing_platform.hooks.campaign_manager import GoogleCampaignManagerHook
from airflow.utils.decorators import apply_defaults
+try:
+ from airflow.providers.google.cloud.hooks.gcs import GCSHook
+except ImportError:
+ warnings.warn("The package google-cloud must be installed to use the operator.",
Review comment:
I agree - but either we decide to move it all to one folder/provider or we leave it as three dependent packages. I do not want to make an exception to the structure of provider packages. It makes it much easier to have one approach where provider package (e.g. google.cloud) is under "providers/google/cloud/" directory and "operators/hooks/sensors" are right directly under it.
This way we do not have to keep any mapping between folders and provider packages - it is straightforward and can be easily derived from where the operators/hooks/sensors folders are (in fact I am already doing it when I check for import-cross-dependencies).
I am ok with both - separate packages and dependencies between them and moving everything back to "google" provider. Alternatively we could have google/common but I think it's the worst idea of the three. I have slight preference for option 1) because with limited risk and effort it will let us explore consequences of having multiple packages with dependencies but 2) is also fine for me.
Whatever we do - I think we should keep consistency between folder structure and "provider packages" structure (provider name is formed from the directories between "providers" and "operators" basically).
What do you think @mik-laj @nuclearpinguin @michalslowikowski00 @ANiteckiP @kaxil @ashb ?
Option 1):
1 single "google" provider package:
```
providers/google/
operators
hooks
sensors
```
Option 2)
3 separate providers: "google.cloud", "google.marketing", "google.suite". Both "gogle.marketing" and "google.suite" is pretty much useless without "google.cloud" (but it will be printed as warning when there is no google.cloud already installed..
```
providers/google/cloud
operators
...
providers/google/marketing
operators
...
providers/google/suite
operators
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on issue #7506: [AIRFLOW-6877] [WIP]
check cross providers dependencies depends on [AIRFLOW-6663]
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #7506: [AIRFLOW-6877] [WIP] check cross providers dependencies depends on [AIRFLOW-6663]
URL: https://github.com/apache/airflow/pull/7506#issuecomment-589978777
@ashb @feluelle @Fokko @mik-laj @nuclearpinguin @kaxil @BasPH -> I think this one is really important to make decision about releasing backport packages for 1.10 and also a good prerequisite for AIP-8.
Happy to hear your comments!
----------------------------------------------------------------
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
With regards,
Apache Git Services