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/11/10 19:28:17 UTC

[GitHub] [airflow] potiuk opened a new pull request #12265: Providers in extras are properly configured and verified

potiuk opened a new pull request #12265:
URL: https://github.com/apache/airflow/pull/12265


   This fixes #12255 - where we published beta2 release with some
   extras pulling non-existing providers.
   
   The exact list of providers that had problems:
   
   Wrongly named extras/providers:
   
   * apache.presto: it was badly named -> renamed to 'presto'
   * spark (badly pointing to spark instead of apache.spark)
   * yandexcloud (the name remains there but we've also added 'yandex' extra to correspond 1-1 with 'yandex' provider
   
   Extras that were wrongly marked as having providers, where they had
   none:
   
   * dask
   * rabbitmq
   * sentry
   * statsd
   * tableau
   * virtualenv
   
   <!--
   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] potiuk commented on pull request #12265: Providers in extras are properly configured and verified

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #12265:
URL: https://github.com/apache/airflow/pull/12265#issuecomment-724917143


   cc: @jedcunningham @mjpieters -> there were few more :(


----------------------------------------------------------------
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 #12265: Providers in extras are properly configured and verified

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #12265:
URL: https://github.com/apache/airflow/pull/12265#issuecomment-725373670


   Fixes applied!


----------------------------------------------------------------
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 #12265: Providers in extras are properly configured and verified

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #12265:
URL: https://github.com/apache/airflow/pull/12265#discussion_r521287434



##########
File path: scripts/ci/pre_commit/pre_commit_check_extras_have_providers.py
##########
@@ -0,0 +1,68 @@
+#!/usr/bin/env python3
+# 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.
+
+# Check if the extras have providers defined.
+import os
+import sys
+from os.path import dirname
+from textwrap import wrap
+from typing import List
+
+AIRFLOW_SOURCES_DIR = os.path.abspath(os.path.join(dirname(__file__), os.pardir, os.pardir, os.pardir))
+
+sys.path.insert(0, AIRFLOW_SOURCES_DIR)
+# flake8: noqa: F401
+# pylint: disable=wrong-import-position
+from setup import EXTRAS_PROVIDERS_PACKAGES  # noqa
+
+sys.path.append(AIRFLOW_SOURCES_DIR)
+
+
+def get_provider_directory(provider: str):
+    return os.path.join(AIRFLOW_SOURCES_DIR, "airflow", "providers", *provider.split('.'))
+
+
+def check_all_providers() -> List[str]:
+    errors: List[str] = []
+    for extra, providers in EXTRAS_PROVIDERS_PACKAGES.items():
+        for provider in providers:
+            provider_directory = get_provider_directory(provider)
+            if not os.path.isdir(provider_directory):
+                errors.append(
+                    f"The {extra} has provider {provider} that has" f" missing {provider_directory} directory"

Review comment:
       Ah. Blacks's joining the lines :)




----------------------------------------------------------------
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 #12265: Providers in extras are properly configured and verified

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12265:
URL: https://github.com/apache/airflow/pull/12265#issuecomment-725445457


   The PR needs to run all tests because it modifies core of Airflow! Please rebase it to latest master or ask committer to re-run 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] kaxil commented on a change in pull request #12265: Providers in extras are properly configured and verified

Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #12265:
URL: https://github.com/apache/airflow/pull/12265#discussion_r520927171



##########
File path: scripts/ci/pre_commit/pre_commit_check_extras_have_providers.py
##########
@@ -0,0 +1,68 @@
+#!/usr/bin/env python3
+# 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.
+
+# Check if the extras have providers defined.
+import os
+import sys
+from os.path import dirname
+from textwrap import wrap
+from typing import List
+
+AIRFLOW_SOURCES_DIR = os.path.abspath(os.path.join(dirname(__file__), os.pardir, os.pardir, os.pardir))
+
+sys.path.insert(0, AIRFLOW_SOURCES_DIR)
+# flake8: noqa: F401
+# pylint: disable=wrong-import-position
+from setup import EXTRAS_PROVIDERS_PACKAGES  # noqa
+
+sys.path.append(AIRFLOW_SOURCES_DIR)
+
+
+def get_provider_directory(provider: str):
+    return os.path.join(AIRFLOW_SOURCES_DIR, "airflow", "providers", *provider.split('.'))
+
+
+def check_all_providers() -> List[str]:
+    errors: List[str] = []
+    for extra, providers in EXTRAS_PROVIDERS_PACKAGES.items():
+        for provider in providers:
+            provider_directory = get_provider_directory(provider)
+            if not os.path.isdir(provider_directory):
+                errors.append(
+                    f"The {extra} has provider {provider} that has" f" missing {provider_directory} directory"

Review comment:
       ```suggestion
                       f"The {extra} has provider {provider} that has missing {provider_directory} directory"
   ```




----------------------------------------------------------------
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 #12265: Providers in extras are properly configured and verified

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #12265:
URL: https://github.com/apache/airflow/pull/12265#discussion_r521287846



##########
File path: scripts/ci/pre_commit/pre_commit_check_extras_have_providers.py
##########
@@ -0,0 +1,68 @@
+#!/usr/bin/env python3
+# 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.
+
+# Check if the extras have providers defined.
+import os
+import sys
+from os.path import dirname
+from textwrap import wrap
+from typing import List
+
+AIRFLOW_SOURCES_DIR = os.path.abspath(os.path.join(dirname(__file__), os.pardir, os.pardir, os.pardir))
+
+sys.path.insert(0, AIRFLOW_SOURCES_DIR)
+# flake8: noqa: F401
+# pylint: disable=wrong-import-position
+from setup import EXTRAS_PROVIDERS_PACKAGES  # noqa
+
+sys.path.append(AIRFLOW_SOURCES_DIR)
+
+
+def get_provider_directory(provider: str):
+    return os.path.join(AIRFLOW_SOURCES_DIR, "airflow", "providers", *provider.split('.'))
+
+
+def check_all_providers() -> List[str]:
+    errors: List[str] = []
+    for extra, providers in EXTRAS_PROVIDERS_PACKAGES.items():
+        for provider in providers:
+            provider_directory = get_provider_directory(provider)
+            if not os.path.isdir(provider_directory):
+                errors.append(
+                    f"The {extra} has provider {provider} that has" f" missing {provider_directory} directory"
+                )
+                continue
+            if not os.path.exists(os.path.join(provider_directory, "__init__.py")):
+                errors.append(
+                    f"The {extra} has provider {provider} that has"
+                    f" missing __init__.py in the {provider_directory} directory"
+                )
+            if not os.path.exists(os.path.join(provider_directory, "README.md")):
+                errors.append(
+                    f"The {extra} has provider {provider} that has"
+                    f" missing README.md in the {provider_directory} directory"
+                )
+    return errors
+
+
+if __name__ == '__main__':
+    errors = check_all_providers()
+    if errors:
+        for message in errors:
+            print(f"{message}", file=sys.stderr)

Review comment:
       Oh yeah. It was more complex before. I have not noticed how much i simplified the f-string :)




----------------------------------------------------------------
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 #12265: Providers in extras are properly configured and verified

Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #12265:
URL: https://github.com/apache/airflow/pull/12265#discussion_r520927392



##########
File path: scripts/ci/pre_commit/pre_commit_check_extras_have_providers.py
##########
@@ -0,0 +1,68 @@
+#!/usr/bin/env python3
+# 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.
+
+# Check if the extras have providers defined.
+import os
+import sys
+from os.path import dirname
+from textwrap import wrap
+from typing import List
+
+AIRFLOW_SOURCES_DIR = os.path.abspath(os.path.join(dirname(__file__), os.pardir, os.pardir, os.pardir))
+
+sys.path.insert(0, AIRFLOW_SOURCES_DIR)
+# flake8: noqa: F401
+# pylint: disable=wrong-import-position
+from setup import EXTRAS_PROVIDERS_PACKAGES  # noqa
+
+sys.path.append(AIRFLOW_SOURCES_DIR)
+
+
+def get_provider_directory(provider: str):
+    return os.path.join(AIRFLOW_SOURCES_DIR, "airflow", "providers", *provider.split('.'))
+
+
+def check_all_providers() -> List[str]:
+    errors: List[str] = []
+    for extra, providers in EXTRAS_PROVIDERS_PACKAGES.items():
+        for provider in providers:
+            provider_directory = get_provider_directory(provider)
+            if not os.path.isdir(provider_directory):
+                errors.append(
+                    f"The {extra} has provider {provider} that has" f" missing {provider_directory} directory"
+                )
+                continue
+            if not os.path.exists(os.path.join(provider_directory, "__init__.py")):
+                errors.append(
+                    f"The {extra} has provider {provider} that has"
+                    f" missing __init__.py in the {provider_directory} directory"
+                )
+            if not os.path.exists(os.path.join(provider_directory, "README.md")):
+                errors.append(
+                    f"The {extra} has provider {provider} that has"
+                    f" missing README.md in the {provider_directory} directory"
+                )
+    return errors
+
+
+if __name__ == '__main__':
+    errors = check_all_providers()
+    if errors:
+        for message in errors:
+            print(f"{message}", file=sys.stderr)

Review comment:
       ```suggestion
               print(message, file=sys.stderr)
   ```




----------------------------------------------------------------
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 merged pull request #12265: Providers in extras are properly configured and verified

Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #12265:
URL: https://github.com/apache/airflow/pull/12265


   


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