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 2021/01/02 15:50:59 UTC

[GitHub] [airflow] potiuk opened a new pull request #13439: Forces unistalling providers in editable mode.

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


   We cannot skip installing providers, but this causes
   problems when installing airflow in editable mode, because providers
   are in two places - in airflow sources and in provider packages.
   
   This change removes installed provider packages when airflow
   is installed in editable mode to mitigate the problem.
   
   This way, there is no need to use INSTALL_PROVIDERS_FROM_SOURCES
   variable when installing in editable mode.
   
   We still need to keep INSTALL_PROVIDERS_FROM_SOURCES for cases when
   non-editable mode is used. In this way one can easily install curent
   version of provider packages locally with pip install and have
   the latest sources of both airflow and providers installed.
   
   <!--
   Thank you for contributing! Please make sure that your code changes
   are covered with tests. And in case of new features or big changes
   remember to adjust the documentation.
   
   Feel free to ping committers for the review!
   
   In case of existing issue, reference it using one of the following:
   
   closes: #ISSUE
   related: #ISSUE
   
   How to write a good git commit message:
   http://chris.beams.io/posts/git-commit/
   -->
   
   ---
   **^ Add meaningful description above**
   
   Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
   In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
   In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
   In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] github-actions[bot] commented on pull request #13439: Forces unistalling providers in editable mode.

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


   [The Workflow run](https://github.com/apache/airflow/actions/runs/457889862) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] kaxil commented on a change in pull request #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -780,76 +826,135 @@ def get_provider_package_from_package_id(package_id: str):
 
 
 class AirflowDistribution(Distribution):
-    """setuptools.Distribution subclass with Airflow specific behaviour"""
+    """
+    The setuptools.Distribution subclass with Airflow specific behaviour
+
+    The reason for pylint: disable=signature-differs of parse_config_files is explained here:
+    https://github.com/PyCQA/pylint/issues/3737
+
+    """
 
-    # https://github.com/PyCQA/pylint/issues/3737
     def parse_config_files(self, *args, **kwargs):  # pylint: disable=signature-differs
         """
         Ensure that when we have been asked to install providers from sources
-        that we don't *also* try to install those providers from PyPI
+        that we don't *also* try to install those providers from PyPI.
+        Also we should make sure that in this case we copy provider.yaml files so that
+        Providers manager can find package information.
         """
         super().parse_config_files(*args, **kwargs)
-        if os.getenv('INSTALL_PROVIDERS_FROM_SOURCES') == 'true':
+        if os.getenv(INSTALL_PROVIDERS_FROM_SOURCES) == 'true':
             self.install_requires = [  # noqa  pylint: disable=attribute-defined-outside-init
                 req for req in self.install_requires if not req.startswith('apache-airflow-providers-')
             ]
+            provider_yaml_files = glob.glob("airflow/providers/**/provider.yaml", recursive=True)
+            for provider_yaml_file in provider_yaml_files:
+                provider_relative_path = relpath(provider_yaml_file, os.path.join(my_dir, "airflow"))
+                self.package_data['airflow'].append(provider_relative_path)
         else:
             self.install_requires.extend(
                 [get_provider_package_from_package_id(package_id) for package_id in PREINSTALLED_PROVIDERS]
             )
 
 
-def add_provider_packages_to_requirements(extra_with_providers: str, providers: List[str]):
+def add_provider_packages_to_extras_requirements(extra: str, providers: List[str]) -> None:
     """
-    Adds provider packages to requirements
+    Adds provider packages to requirements of extra.
 
-    :param extra_with_providers: Name of the extra to add providers to
-    :param providers: list of provider names
+    :param extra: Name of the extra to add providers to
+    :param providers: list of provider ids
     """
-    EXTRAS_WITH_PROVIDERS.add(extra_with_providers)
-    EXTRAS_REQUIREMENTS[extra_with_providers].extend(
+    EXTRAS_WITH_PROVIDERS.add(extra)
+    EXTRAS_REQUIREMENTS[extra].extend(
         [get_provider_package_from_package_id(package_name) for package_name in providers]
     )
 
 
-def add_all_provider_packages():
+def add_all_provider_packages() -> None:
     """
-    In case of regular installation (when INSTALL_PROVIDERS_FROM_SOURCES is false), we should
-    add extra dependencies to Airflow - to get the providers automatically installed when
-    those extras are installed.
+    In case of regular installation (providers installed from packages), we should add extra dependencies to
+    Airflow - to get the providers automatically installed when those extras are installed.
+
+    For providers installed from sources we skip that step. That helps to test and install airflow with
+    all packages in CI - for example when new providers are added, otherwise the installation would fail
+    as the new provider is not yet in PyPI.
 
     """
     for provider in ALL_PROVIDERS:
-        add_provider_packages_to_requirements(provider, [provider])
-    add_provider_packages_to_requirements("all", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("devel_ci", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("devel_all", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("all_dbs", ALL_DB_PROVIDERS)
-    add_provider_packages_to_requirements("devel_hadoop", ["apache.hdfs", "apache.hive", "presto"])
+        add_provider_packages_to_extras_requirements(provider, [provider])
+    add_provider_packages_to_extras_requirements("all", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_ci", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_all", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("all_dbs", ALL_DB_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_hadoop", ["apache.hdfs", "apache.hive", "presto"])
+
+
+class Develop(develop_orig):
+    """Forces removal of providers in editable mode."""
+
+    def run(self):
+        self.announce('Installing in editable mode. Uninstalling provider packages!', level=log.INFO)
+        # We need to run "python3 -m pip" because it might be that older PIP binary is in the path
+        # And it results with an error when running pip directly (cannot import pip module)
+        # also PIP does not have a stable API so we have to run subprocesses ¯\_(ツ)_/¯
+        try:
+            installed_packages = (
+                subprocess.check_output(["python3", "-m", "pip", "freeze"]).decode().splitlines()
+            )
+            airflow_provider_packages = [
+                package_line.split("=")[0]
+                for package_line in installed_packages
+                if package_line.startswith("apache-airflow-providers")
+            ]
+            self.announce(f'Uninstalling ${airflow_provider_packages}!', level=log.INFO)
+            subprocess.check_call(["python3", "-m", "pip", "uninstall", "--yes", *airflow_provider_packages])
+        except subprocess.CalledProcessError as e:
+            self.announce(f'Error when uninstalling airflow provider packages: {e}!', level=log.WARN)
+        super().run()
+
+
+class Install(install_orig):
+    """Forces installation of providers from sources in editable mode."""
 
+    def run(self):
+        self.announce('Standard installation. Providers are installed from packages', level=log.INFO)
+        super().run()
 
-def do_setup():
-    """Perform the Airflow package setup."""
+
+def do_setup() -> None:
+    """
+    Perform the Airflow package setup.
+    Most values come from setup.cfg, only the dynamically calculated ones are passed to setup
+    function call. See https://setuptools.readthedocs.io/en/latest/userguide/declarative_config.html
+    """
     setup_kwargs = {}
 
-    if os.getenv('INSTALL_PROVIDERS_FROM_SOURCES') == 'true':
-        # Only specify this if we need this option, otherwise let default from
-        # setup.cfg control this (kwargs in setup() call take priority)
-        setup_kwargs['packages'] = find_namespace_packages(include=['airflow*'])
+    def include_provider_namespace_packages_when_installing_from_sources() -> None:

Review comment:
       Why is this a separate function?




----------------------------------------------------------------
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] turbaszek commented on a change in pull request #13439: Forces unistalling providers in editable mode.

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



##########
File path: INSTALL
##########
@@ -51,24 +51,41 @@ pip install . \
   --constraint "https://raw.githubusercontent.com/apache/airflow/constraints-master/constraints-3.6.txt"
 
 
-By default `pip install` in Airflow 2.0 installs only the provider packages that are needed by the extras,
-however if you want to install all providers (which was default behaviour in 1.10.*)
-you can do it by setting environment variable INSTALL_PROVIDERS_FROM_SOURCES to `true`.
+By default `pip install` in Airflow 2.0 installs only the provider packages that are needed by the extras and
+install them as packages from PyPI rather than from local sources:
 
 
-INSTALL_PROVIDERS_FROM_SOURCES="true" pip install . \
+pip install . \
   --constraint "https://raw.githubusercontent.com/apache/airflow/constraints-master/constraints-3.6.txt"
 
 
-You can also install airflow in "editable mode" (with -e) flag and then provider packages will be
-available, because they are used directly from the airflow sources:
+You can also install airflow in "editable mode" (with -e) flag and then provider packages are
+available directly from the sources (and the provider packages installed from PyPI are UNINSTALLED in
+order to avoid having providers in two places. And `provider.yaml` files are used to discover capabilities
+of the providers which are part of the airflow source code.
+
+You can read more about `provider.yaml` and community-managed providers in
+http://airflow.apache.org/docs/apache-airflow-providers/index.html

Review comment:
       ```suggestion
   https://airflow.apache.org/docs/apache-airflow-providers/index.html
   ```




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -150,13 +162,22 @@ def write_version(filename: str = os.path.join(*[my_dir, "airflow", "git_version
         file.write(text)
 
 
-if os.environ.get('USE_THEME_FROM_GIT'):
-    _SPHINX_AIRFLOW_THEME_URL = (
-        "@ https://github.com/apache/airflow-site/releases/download/0.0.4/"
-        "sphinx_airflow_theme-0.0.4-py3-none-any.whl"
-    )
-else:
-    _SPHINX_AIRFLOW_THEME_URL = ''
+def get_sphinx_theme_version() -> str:
+    """
+    Return sphinx theme version. If USE_THEME_FROM_GIT env variable is set, the theme is used from
+    github tp allow dynamically update it during development. However for regular PIP release

Review comment:
       ```suggestion
       GitHub tp allow dynamically update it during development. However for regular PIP release
   ```

##########
File path: setup.py
##########
@@ -150,13 +162,22 @@ def write_version(filename: str = os.path.join(*[my_dir, "airflow", "git_version
         file.write(text)
 
 
-if os.environ.get('USE_THEME_FROM_GIT'):
-    _SPHINX_AIRFLOW_THEME_URL = (
-        "@ https://github.com/apache/airflow-site/releases/download/0.0.4/"
-        "sphinx_airflow_theme-0.0.4-py3-none-any.whl"
-    )
-else:
-    _SPHINX_AIRFLOW_THEME_URL = ''
+def get_sphinx_theme_version() -> str:
+    """
+    Return sphinx theme version. If USE_THEME_FROM_GIT env variable is set, the theme is used from
+    github tp allow dynamically update it during development. However for regular PIP release

Review comment:
       ```suggestion
       GitHub to allow dynamically update it during development. However for regular PIP release
   ```




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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



##########
File path: LOCAL_VIRTUALENV.rst
##########
@@ -135,17 +134,40 @@ To create and initialize the local virtualenv:
 
    .. code-block:: bash
 
-    pip install -U -e ".[devel,<OTHER EXTRAS>]" # for example: pip install -U -e ".[devel,google,postgres]"
+    pip install --upgrade -e ".[devel,<OTHER EXTRAS>]" # for example: pip install --upgrade -e ".[devel,google,postgres]"
 
 In case you have problems with installing airflow because of some requirements are not installable, you can
 try to install it with the set of working constraints (note that there are different constraint files
 for different python versions:
 
    .. code-block:: bash
 
-    pip install -U -e ".[devel,<OTHER EXTRAS>]" \
+    pip install -e ".[devel,<OTHER EXTRAS>]" \
+        --constraint "https://raw.githubusercontent.com/apache/airflow/constraints-master/constraints-3.6.txt"
+
+
+This will install Airflow in 'editable' mode - where sources of Airflow are taken directly from the source
+code rather than moved to the installation directory. During the installation airflow will install - but then
+automatically remove all provider packages installed from pypi - instead it will automatically use the
+provider packages available in your local sources.

Review comment:
       ```suggestion
   This will install Airflow in 'editable' mode - where sources of Airflow are taken directly from the source
   code rather than moved to the installation directory. During the installation airflow will install - but then
   automatically remove all provider packages installed from PyPI - instead it will automatically use the
   provider packages available in your local sources.
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on a change in pull request #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -150,13 +162,22 @@ def write_version(filename: str = os.path.join(*[my_dir, "airflow", "git_version
         file.write(text)
 
 
-if os.environ.get('USE_THEME_FROM_GIT'):
-    _SPHINX_AIRFLOW_THEME_URL = (
-        "@ https://github.com/apache/airflow-site/releases/download/0.0.4/"
-        "sphinx_airflow_theme-0.0.4-py3-none-any.whl"
-    )
-else:
-    _SPHINX_AIRFLOW_THEME_URL = ''
+def get_sphinx_theme_version() -> str:
+    """
+    Return sphinx theme version. If USE_THEME_FROM_GIT env variable is set, the theme is used from
+    github tp allow dynamically update it during development. However for regular PIP release
+    you cannot use @ package specification, so the latest available released theme package from
+    PIP is used.
+    :return: the

Review comment:
       Will change!




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -150,13 +162,22 @@ def write_version(filename: str = os.path.join(*[my_dir, "airflow", "git_version
         file.write(text)
 
 
-if os.environ.get('USE_THEME_FROM_GIT'):
-    _SPHINX_AIRFLOW_THEME_URL = (
-        "@ https://github.com/apache/airflow-site/releases/download/0.0.4/"
-        "sphinx_airflow_theme-0.0.4-py3-none-any.whl"
-    )
-else:
-    _SPHINX_AIRFLOW_THEME_URL = ''
+def get_sphinx_theme_version() -> str:
+    """
+    Return sphinx theme version. If USE_THEME_FROM_GIT env variable is set, the theme is used from
+    github tp allow dynamically update it during development. However for regular PIP release
+    you cannot use @ package specification, so the latest available released theme package from
+    PIP is used.
+    :return: the
+    """
+    if os.environ.get('USE_THEME_FROM_GIT'):
+        return (
+            "@ https://github.com/apache/airflow-site/releases/download/0.0.4/"
+            + "sphinx_airflow_theme-0.0.4-py3-none-any.whl"

Review comment:
       We added pylint rule to not allow implicit concatenation so the + should be there (but that makes me think that the #noqa suppressed that one as well so I will double check) .




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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



##########
File path: INSTALL
##########
@@ -51,24 +51,35 @@ pip install . \
   --constraint "https://raw.githubusercontent.com/apache/airflow/constraints-master/constraints-3.6.txt"
 
 
-By default `pip install` in Airflow 2.0 installs only the provider packages that are needed by the extras,
-however if you want to install all providers (which was default behaviour in 1.10.*)
-you can do it by setting environment variable INSTALL_PROVIDERS_FROM_SOURCES to `true`.
+By default `pip install` in Airflow 2.0 installs only the provider packages that are needed by the extras and
+install them as packages from PyPI rather than from local sources:
 
 
-INSTALL_PROVIDERS_FROM_SOURCES="true" pip install . \
+pip install . \
   --constraint "https://raw.githubusercontent.com/apache/airflow/constraints-master/constraints-3.6.txt"
 
 
-You can also install airflow in "editable mode" (with -e) flag and then provider packages will be
-available, because they are used directly from the airflow sources:
-
+You can also install airflow in "editable mode" (with -e) flag and then provider packages are
+available directly from the sources (and the provider packages installed from PyPI are uninstalled in
+order to avoid having providers in two places. This is useful if you want to develop providers:
 
 pip install -e . \
   --constraint "https://raw.githubusercontent.com/apache/airflow/constraints-master/constraints-3.6.txt"
 
+You can als skip installing provider packages from PyPI by setting INSTALL_PROVIDERS_FROM_SOURCE to "true".
+In this case Airflow will be installed in non-editable mode with all providers installed from the sources.
+Additionally provider.yaml files will also be copied to providers folders which will make the providers

Review comment:
       Nope. It's provider.yaml




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] ashb commented on a change in pull request #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -780,76 +826,135 @@ def get_provider_package_from_package_id(package_id: str):
 
 
 class AirflowDistribution(Distribution):
-    """setuptools.Distribution subclass with Airflow specific behaviour"""
+    """
+    The setuptools.Distribution subclass with Airflow specific behaviour
+
+    The reason for pylint: disable=signature-differs of parse_config_files is explained here:
+    https://github.com/PyCQA/pylint/issues/3737
+
+    """
 
-    # https://github.com/PyCQA/pylint/issues/3737
     def parse_config_files(self, *args, **kwargs):  # pylint: disable=signature-differs
         """
         Ensure that when we have been asked to install providers from sources
-        that we don't *also* try to install those providers from PyPI
+        that we don't *also* try to install those providers from PyPI.
+        Also we should make sure that in this case we copy provider.yaml files so that
+        Providers manager can find package information.
         """
         super().parse_config_files(*args, **kwargs)
-        if os.getenv('INSTALL_PROVIDERS_FROM_SOURCES') == 'true':
+        if os.getenv(INSTALL_PROVIDERS_FROM_SOURCES) == 'true':
             self.install_requires = [  # noqa  pylint: disable=attribute-defined-outside-init
                 req for req in self.install_requires if not req.startswith('apache-airflow-providers-')
             ]
+            provider_yaml_files = glob.glob("airflow/providers/**/provider.yaml", recursive=True)
+            for provider_yaml_file in provider_yaml_files:
+                provider_relative_path = relpath(provider_yaml_file, os.path.join(my_dir, "airflow"))
+                self.package_data['airflow'].append(provider_relative_path)
         else:
             self.install_requires.extend(
                 [get_provider_package_from_package_id(package_id) for package_id in PREINSTALLED_PROVIDERS]
             )
 
 
-def add_provider_packages_to_requirements(extra_with_providers: str, providers: List[str]):
+def add_provider_packages_to_extras_requirements(extra: str, providers: List[str]) -> None:
     """
-    Adds provider packages to requirements
+    Adds provider packages to requirements of extra.
 
-    :param extra_with_providers: Name of the extra to add providers to
-    :param providers: list of provider names
+    :param extra: Name of the extra to add providers to
+    :param providers: list of provider ids
     """
-    EXTRAS_WITH_PROVIDERS.add(extra_with_providers)
-    EXTRAS_REQUIREMENTS[extra_with_providers].extend(
+    EXTRAS_WITH_PROVIDERS.add(extra)
+    EXTRAS_REQUIREMENTS[extra].extend(
         [get_provider_package_from_package_id(package_name) for package_name in providers]
     )
 
 
-def add_all_provider_packages():
+def add_all_provider_packages() -> None:
     """
-    In case of regular installation (when INSTALL_PROVIDERS_FROM_SOURCES is false), we should
-    add extra dependencies to Airflow - to get the providers automatically installed when
-    those extras are installed.
+    In case of regular installation (providers installed from packages), we should add extra dependencies to
+    Airflow - to get the providers automatically installed when those extras are installed.
+
+    For providers installed from sources we skip that step. That helps to test and install airflow with
+    all packages in CI - for example when new providers are added, otherwise the installation would fail
+    as the new provider is not yet in PyPI.
 
     """
     for provider in ALL_PROVIDERS:
-        add_provider_packages_to_requirements(provider, [provider])
-    add_provider_packages_to_requirements("all", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("devel_ci", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("devel_all", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("all_dbs", ALL_DB_PROVIDERS)
-    add_provider_packages_to_requirements("devel_hadoop", ["apache.hdfs", "apache.hive", "presto"])
+        add_provider_packages_to_extras_requirements(provider, [provider])
+    add_provider_packages_to_extras_requirements("all", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_ci", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_all", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("all_dbs", ALL_DB_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_hadoop", ["apache.hdfs", "apache.hive", "presto"])
+
+
+class Develop(develop_orig):
+    """Forces removal of providers in editable mode."""
+
+    def run(self):
+        self.announce('Installing in editable mode. Uninstalling provider packages!', level=log.INFO)
+        # We need to run "python3 -m pip" because it might be that older PIP binary is in the path
+        # And it results with an error when running pip directly (cannot import pip module)
+        # also PIP does not have a stable API so we have to run subprocesses ¯\_(ツ)_/¯
+        try:
+            installed_packages = (
+                subprocess.check_output(["python3", "-m", "pip", "freeze"]).decode().splitlines()
+            )
+            airflow_provider_packages = [
+                package_line.split("=")[0]
+                for package_line in installed_packages
+                if package_line.startswith("apache-airflow-providers")
+            ]
+            self.announce(f'Uninstalling ${airflow_provider_packages}!', level=log.INFO)
+            subprocess.check_call(["python3", "-m", "pip", "uninstall", "--yes", *airflow_provider_packages])
+        except subprocess.CalledProcessError as e:
+            self.announce(f'Error when uninstalling airflow provider packages: {e}!', level=log.WARN)
+        super().run()
+
+
+class Install(install_orig):
+    """Forces installation of providers from sources in editable mode."""
 
+    def run(self):
+        self.announce('Standard installation. Providers are installed from packages', level=log.INFO)
+        super().run()
 
-def do_setup():
-    """Perform the Airflow package setup."""
+
+def do_setup() -> None:
+    """
+    Perform the Airflow package setup.
+    Most values come from setup.cfg, only the dynamically calculated ones are passed to setup
+    function call. See https://setuptools.readthedocs.io/en/latest/userguide/declarative_config.html
+    """
     setup_kwargs = {}
 
-    if os.getenv('INSTALL_PROVIDERS_FROM_SOURCES') == 'true':
-        # Only specify this if we need this option, otherwise let default from
-        # setup.cfg control this (kwargs in setup() call take priority)
-        setup_kwargs['packages'] = find_namespace_packages(include=['airflow*'])
+    def include_provider_namespace_packages_when_installing_from_sources() -> None:
+        """
+        When install providers from sources we install all namespace packages found below airflow,
+        including airflow and provider packages, otherwise defaults from setup.cfg control this.
+        The kwargs in setup() call override those that are specified in setup.cfg.
+        """
+        if os.getenv(INSTALL_PROVIDERS_FROM_SOURCES) == 'true':
+            setup_kwargs['packages'] = find_namespace_packages(include=['airflow*'])
+
+    include_provider_namespace_packages_when_installing_from_sources()
+    if os.getenv(INSTALL_PROVIDERS_FROM_SOURCES) == 'true':
+        print("Installing providers from sources. Skip adding providers as dependencies")
     else:
         add_all_provider_packages()
+
     write_version()
     setup(
         distclass=AirflowDistribution,
-        # Most values come from setup.cfg -- see
-        # https://setuptools.readthedocs.io/en/latest/userguide/declarative_config.html

Review comment:
       Did you mean to remove this comment?




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -150,13 +162,22 @@ def write_version(filename: str = os.path.join(*[my_dir, "airflow", "git_version
         file.write(text)
 
 
-if os.environ.get('USE_THEME_FROM_GIT'):
-    _SPHINX_AIRFLOW_THEME_URL = (
-        "@ https://github.com/apache/airflow-site/releases/download/0.0.4/"
-        "sphinx_airflow_theme-0.0.4-py3-none-any.whl"
-    )
-else:
-    _SPHINX_AIRFLOW_THEME_URL = ''
+def get_sphinx_theme_version() -> str:
+    """
+    Return sphinx theme version. If USE_THEME_FROM_GIT env variable is set, the theme is used from
+    github tp allow dynamically update it during development. However for regular PIP release
+    you cannot use @ package specification, so the latest available released theme package from
+    PIP is used.
+    :return: the
+    """
+    if os.environ.get('USE_THEME_FROM_GIT'):
+        return (
+            "@ https://github.com/apache/airflow-site/releases/download/0.0.4/"
+            + "sphinx_airflow_theme-0.0.4-py3-none-any.whl"
+        )
+    else:
+        return ''

Review comment:
       ```suggestion
       return ''
   ```
   
   `else` is not required here as if the first condition is True, it will return




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -780,76 +826,135 @@ def get_provider_package_from_package_id(package_id: str):
 
 
 class AirflowDistribution(Distribution):
-    """setuptools.Distribution subclass with Airflow specific behaviour"""
+    """
+    The setuptools.Distribution subclass with Airflow specific behaviour
+
+    The reason for pylint: disable=signature-differs of parse_config_files is explained here:
+    https://github.com/PyCQA/pylint/issues/3737
+
+    """
 
-    # https://github.com/PyCQA/pylint/issues/3737
     def parse_config_files(self, *args, **kwargs):  # pylint: disable=signature-differs
         """
         Ensure that when we have been asked to install providers from sources
-        that we don't *also* try to install those providers from PyPI
+        that we don't *also* try to install those providers from PyPI.
+        Also we should make sure that in this case we copy provider.yaml files so that
+        Providers manager can find package information.
         """
         super().parse_config_files(*args, **kwargs)
-        if os.getenv('INSTALL_PROVIDERS_FROM_SOURCES') == 'true':
+        if os.getenv(INSTALL_PROVIDERS_FROM_SOURCES) == 'true':
             self.install_requires = [  # noqa  pylint: disable=attribute-defined-outside-init
                 req for req in self.install_requires if not req.startswith('apache-airflow-providers-')
             ]
+            provider_yaml_files = glob.glob("airflow/providers/**/provider.yaml", recursive=True)
+            for provider_yaml_file in provider_yaml_files:
+                provider_relative_path = relpath(provider_yaml_file, os.path.join(my_dir, "airflow"))
+                self.package_data['airflow'].append(provider_relative_path)
         else:
             self.install_requires.extend(
                 [get_provider_package_from_package_id(package_id) for package_id in PREINSTALLED_PROVIDERS]
             )
 
 
-def add_provider_packages_to_requirements(extra_with_providers: str, providers: List[str]):
+def add_provider_packages_to_extras_requirements(extra: str, providers: List[str]) -> None:
     """
-    Adds provider packages to requirements
+    Adds provider packages to requirements of extra.
 
-    :param extra_with_providers: Name of the extra to add providers to
-    :param providers: list of provider names
+    :param extra: Name of the extra to add providers to
+    :param providers: list of provider ids
     """
-    EXTRAS_WITH_PROVIDERS.add(extra_with_providers)
-    EXTRAS_REQUIREMENTS[extra_with_providers].extend(
+    EXTRAS_WITH_PROVIDERS.add(extra)
+    EXTRAS_REQUIREMENTS[extra].extend(
         [get_provider_package_from_package_id(package_name) for package_name in providers]
     )
 
 
-def add_all_provider_packages():
+def add_all_provider_packages() -> None:
     """
-    In case of regular installation (when INSTALL_PROVIDERS_FROM_SOURCES is false), we should
-    add extra dependencies to Airflow - to get the providers automatically installed when
-    those extras are installed.
+    In case of regular installation (providers installed from packages), we should add extra dependencies to
+    Airflow - to get the providers automatically installed when those extras are installed.
+
+    For providers installed from sources we skip that step. That helps to test and install airflow with
+    all packages in CI - for example when new providers are added, otherwise the installation would fail
+    as the new provider is not yet in PyPI.
 
     """
     for provider in ALL_PROVIDERS:
-        add_provider_packages_to_requirements(provider, [provider])
-    add_provider_packages_to_requirements("all", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("devel_ci", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("devel_all", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("all_dbs", ALL_DB_PROVIDERS)
-    add_provider_packages_to_requirements("devel_hadoop", ["apache.hdfs", "apache.hive", "presto"])
+        add_provider_packages_to_extras_requirements(provider, [provider])
+    add_provider_packages_to_extras_requirements("all", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_ci", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_all", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("all_dbs", ALL_DB_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_hadoop", ["apache.hdfs", "apache.hive", "presto"])
+
+
+class Develop(develop_orig):
+    """Forces removal of providers in editable mode."""
+
+    def run(self):
+        self.announce('Installing in editable mode. Uninstalling provider packages!', level=log.INFO)
+        # We need to run "python3 -m pip" because it might be that older PIP binary is in the path
+        # And it results with an error when running pip directly (cannot import pip module)
+        # also PIP does not have a stable API so we have to run subprocesses ¯\_(ツ)_/¯
+        try:
+            installed_packages = (
+                subprocess.check_output(["python3", "-m", "pip", "freeze"]).decode().splitlines()
+            )
+            airflow_provider_packages = [
+                package_line.split("=")[0]
+                for package_line in installed_packages
+                if package_line.startswith("apache-airflow-providers")
+            ]
+            self.announce(f'Uninstalling ${airflow_provider_packages}!', level=log.INFO)
+            subprocess.check_call(["python3", "-m", "pip", "uninstall", "--yes", *airflow_provider_packages])
+        except subprocess.CalledProcessError as e:
+            self.announce(f'Error when uninstalling airflow provider packages: {e}!', level=log.WARN)
+        super().run()
+
+
+class Install(install_orig):
+    """Forces installation of providers from sources in editable mode."""
 
+    def run(self):
+        self.announce('Standard installation. Providers are installed from packages', level=log.INFO)
+        super().run()
 
-def do_setup():
-    """Perform the Airflow package setup."""
+
+def do_setup() -> None:
+    """
+    Perform the Airflow package setup.
+    Most values come from setup.cfg, only the dynamically calculated ones are passed to setup
+    function call. See https://setuptools.readthedocs.io/en/latest/userguide/declarative_config.html
+    """
     setup_kwargs = {}
 
-    if os.getenv('INSTALL_PROVIDERS_FROM_SOURCES') == 'true':
-        # Only specify this if we need this option, otherwise let default from
-        # setup.cfg control this (kwargs in setup() call take priority)
-        setup_kwargs['packages'] = find_namespace_packages(include=['airflow*'])
+    def include_provider_namespace_packages_when_installing_from_sources() -> None:
+        """
+        When install providers from sources we install all namespace packages found below airflow,

Review comment:
       +




----------------------------------------------------------------
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] turbaszek commented on a change in pull request #13439: Forces unistalling providers in editable mode.

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



##########
File path: INSTALL
##########
@@ -51,24 +51,35 @@ pip install . \
   --constraint "https://raw.githubusercontent.com/apache/airflow/constraints-master/constraints-3.6.txt"
 
 
-By default `pip install` in Airflow 2.0 installs only the provider packages that are needed by the extras,
-however if you want to install all providers (which was default behaviour in 1.10.*)
-you can do it by setting environment variable INSTALL_PROVIDERS_FROM_SOURCES to `true`.
+By default `pip install` in Airflow 2.0 installs only the provider packages that are needed by the extras and
+install them as packages from PyPI rather than from local sources:
 
 
-INSTALL_PROVIDERS_FROM_SOURCES="true" pip install . \
+pip install . \
   --constraint "https://raw.githubusercontent.com/apache/airflow/constraints-master/constraints-3.6.txt"
 
 
-You can also install airflow in "editable mode" (with -e) flag and then provider packages will be
-available, because they are used directly from the airflow sources:
-
+You can also install airflow in "editable mode" (with -e) flag and then provider packages are
+available directly from the sources (and the provider packages installed from PyPI are uninstalled in

Review comment:
       ```suggestion
   available directly from the sources (and the provider packages installed from PyPI are UNINSTALLED in
   ```
   
   I have a feeling that we should put strong emphasis on this one. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] github-actions[bot] commented on pull request #13439: Forces unistalling providers in editable mode.

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


   [The Workflow run](https://github.com/apache/airflow/actions/runs/458203040) is cancelling this PR. Building image for the PR has been cancelled


----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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



##########
File path: CONTRIBUTING.rst
##########
@@ -712,6 +712,44 @@ snowflake                  slack
 
   .. END PACKAGE DEPENDENCIES HERE
 
+
+Developing provider packages
+----------------------------
+
+While you can develop your own providers, Apache Airflow has 60+ providers that are managed by the community.
+They are part of the same repository as Apache Airflow (we use ``monorepo`` approach where different
+parts of the system are developed in the same repository but then they are packaged and released separately).
+All the community-managed providers are in 'airflow/providers' folder and they are all sub-packages of
+'airflow.providers' package. All the providers are available as ``apache-airflow-providers-<PROVIDER_ID>``
+packages.
+
+The capabilities of the community-managed providers are the same as the third-party ones are the same. When

Review comment:
       ```suggestion
   The capabilities of the community-managed providers are the same as the third-party ones. When
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] ashb commented on a change in pull request #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -780,76 +826,135 @@ def get_provider_package_from_package_id(package_id: str):
 
 
 class AirflowDistribution(Distribution):
-    """setuptools.Distribution subclass with Airflow specific behaviour"""
+    """
+    The setuptools.Distribution subclass with Airflow specific behaviour
+
+    The reason for pylint: disable=signature-differs of parse_config_files is explained here:
+    https://github.com/PyCQA/pylint/issues/3737
+
+    """
 
-    # https://github.com/PyCQA/pylint/issues/3737
     def parse_config_files(self, *args, **kwargs):  # pylint: disable=signature-differs
         """
         Ensure that when we have been asked to install providers from sources
-        that we don't *also* try to install those providers from PyPI
+        that we don't *also* try to install those providers from PyPI.
+        Also we should make sure that in this case we copy provider.yaml files so that
+        Providers manager can find package information.
         """
         super().parse_config_files(*args, **kwargs)
-        if os.getenv('INSTALL_PROVIDERS_FROM_SOURCES') == 'true':
+        if os.getenv(INSTALL_PROVIDERS_FROM_SOURCES) == 'true':
             self.install_requires = [  # noqa  pylint: disable=attribute-defined-outside-init
                 req for req in self.install_requires if not req.startswith('apache-airflow-providers-')
             ]
+            provider_yaml_files = glob.glob("airflow/providers/**/provider.yaml", recursive=True)
+            for provider_yaml_file in provider_yaml_files:
+                provider_relative_path = relpath(provider_yaml_file, os.path.join(my_dir, "airflow"))
+                self.package_data['airflow'].append(provider_relative_path)
         else:
             self.install_requires.extend(
                 [get_provider_package_from_package_id(package_id) for package_id in PREINSTALLED_PROVIDERS]
             )
 
 
-def add_provider_packages_to_requirements(extra_with_providers: str, providers: List[str]):
+def add_provider_packages_to_extras_requirements(extra: str, providers: List[str]) -> None:
     """
-    Adds provider packages to requirements
+    Adds provider packages to requirements of extra.
 
-    :param extra_with_providers: Name of the extra to add providers to
-    :param providers: list of provider names
+    :param extra: Name of the extra to add providers to
+    :param providers: list of provider ids
     """
-    EXTRAS_WITH_PROVIDERS.add(extra_with_providers)
-    EXTRAS_REQUIREMENTS[extra_with_providers].extend(
+    EXTRAS_WITH_PROVIDERS.add(extra)
+    EXTRAS_REQUIREMENTS[extra].extend(
         [get_provider_package_from_package_id(package_name) for package_name in providers]
     )
 
 
-def add_all_provider_packages():
+def add_all_provider_packages() -> None:
     """
-    In case of regular installation (when INSTALL_PROVIDERS_FROM_SOURCES is false), we should
-    add extra dependencies to Airflow - to get the providers automatically installed when
-    those extras are installed.
+    In case of regular installation (providers installed from packages), we should add extra dependencies to
+    Airflow - to get the providers automatically installed when those extras are installed.
+
+    For providers installed from sources we skip that step. That helps to test and install airflow with
+    all packages in CI - for example when new providers are added, otherwise the installation would fail
+    as the new provider is not yet in PyPI.
 
     """
     for provider in ALL_PROVIDERS:
-        add_provider_packages_to_requirements(provider, [provider])
-    add_provider_packages_to_requirements("all", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("devel_ci", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("devel_all", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("all_dbs", ALL_DB_PROVIDERS)
-    add_provider_packages_to_requirements("devel_hadoop", ["apache.hdfs", "apache.hive", "presto"])
+        add_provider_packages_to_extras_requirements(provider, [provider])
+    add_provider_packages_to_extras_requirements("all", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_ci", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_all", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("all_dbs", ALL_DB_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_hadoop", ["apache.hdfs", "apache.hive", "presto"])
+
+
+class Develop(develop_orig):
+    """Forces removal of providers in editable mode."""
+
+    def run(self):
+        self.announce('Installing in editable mode. Uninstalling provider packages!', level=log.INFO)
+        # We need to run "python3 -m pip" because it might be that older PIP binary is in the path
+        # And it results with an error when running pip directly (cannot import pip module)
+        # also PIP does not have a stable API so we have to run subprocesses ¯\_(ツ)_/¯
+        try:
+            installed_packages = (
+                subprocess.check_output(["python3", "-m", "pip", "freeze"]).decode().splitlines()
+            )
+            airflow_provider_packages = [
+                package_line.split("=")[0]
+                for package_line in installed_packages
+                if package_line.startswith("apache-airflow-providers")
+            ]
+            self.announce(f'Uninstalling ${airflow_provider_packages}!', level=log.INFO)
+            subprocess.check_call(["python3", "-m", "pip", "uninstall", "--yes", *airflow_provider_packages])
+        except subprocess.CalledProcessError as e:
+            self.announce(f'Error when uninstalling airflow provider packages: {e}!', level=log.WARN)
+        super().run()
+
+
+class Install(install_orig):
+    """Forces installation of providers from sources in editable mode."""
 
+    def run(self):
+        self.announce('Standard installation. Providers are installed from packages', level=log.INFO)

Review comment:
       This would show up to any user who installs an sdist, so I don't think we should show this here.




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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


   All should be fixed now.


----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -780,76 +826,135 @@ def get_provider_package_from_package_id(package_id: str):
 
 
 class AirflowDistribution(Distribution):
-    """setuptools.Distribution subclass with Airflow specific behaviour"""
+    """
+    The setuptools.Distribution subclass with Airflow specific behaviour
+
+    The reason for pylint: disable=signature-differs of parse_config_files is explained here:
+    https://github.com/PyCQA/pylint/issues/3737
+
+    """
 
-    # https://github.com/PyCQA/pylint/issues/3737
     def parse_config_files(self, *args, **kwargs):  # pylint: disable=signature-differs
         """
         Ensure that when we have been asked to install providers from sources
-        that we don't *also* try to install those providers from PyPI
+        that we don't *also* try to install those providers from PyPI.
+        Also we should make sure that in this case we copy provider.yaml files so that
+        Providers manager can find package information.
         """
         super().parse_config_files(*args, **kwargs)
-        if os.getenv('INSTALL_PROVIDERS_FROM_SOURCES') == 'true':
+        if os.getenv(INSTALL_PROVIDERS_FROM_SOURCES) == 'true':
             self.install_requires = [  # noqa  pylint: disable=attribute-defined-outside-init
                 req for req in self.install_requires if not req.startswith('apache-airflow-providers-')
             ]
+            provider_yaml_files = glob.glob("airflow/providers/**/provider.yaml", recursive=True)
+            for provider_yaml_file in provider_yaml_files:
+                provider_relative_path = relpath(provider_yaml_file, os.path.join(my_dir, "airflow"))
+                self.package_data['airflow'].append(provider_relative_path)
         else:
             self.install_requires.extend(
                 [get_provider_package_from_package_id(package_id) for package_id in PREINSTALLED_PROVIDERS]
             )
 
 
-def add_provider_packages_to_requirements(extra_with_providers: str, providers: List[str]):
+def add_provider_packages_to_extras_requirements(extra: str, providers: List[str]) -> None:
     """
-    Adds provider packages to requirements
+    Adds provider packages to requirements of extra.
 
-    :param extra_with_providers: Name of the extra to add providers to
-    :param providers: list of provider names
+    :param extra: Name of the extra to add providers to
+    :param providers: list of provider ids
     """
-    EXTRAS_WITH_PROVIDERS.add(extra_with_providers)
-    EXTRAS_REQUIREMENTS[extra_with_providers].extend(
+    EXTRAS_WITH_PROVIDERS.add(extra)
+    EXTRAS_REQUIREMENTS[extra].extend(
         [get_provider_package_from_package_id(package_name) for package_name in providers]
     )
 
 
-def add_all_provider_packages():
+def add_all_provider_packages() -> None:
     """
-    In case of regular installation (when INSTALL_PROVIDERS_FROM_SOURCES is false), we should
-    add extra dependencies to Airflow - to get the providers automatically installed when
-    those extras are installed.
+    In case of regular installation (providers installed from packages), we should add extra dependencies to
+    Airflow - to get the providers automatically installed when those extras are installed.
+
+    For providers installed from sources we skip that step. That helps to test and install airflow with
+    all packages in CI - for example when new providers are added, otherwise the installation would fail
+    as the new provider is not yet in PyPI.
 
     """
     for provider in ALL_PROVIDERS:
-        add_provider_packages_to_requirements(provider, [provider])
-    add_provider_packages_to_requirements("all", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("devel_ci", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("devel_all", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("all_dbs", ALL_DB_PROVIDERS)
-    add_provider_packages_to_requirements("devel_hadoop", ["apache.hdfs", "apache.hive", "presto"])
+        add_provider_packages_to_extras_requirements(provider, [provider])
+    add_provider_packages_to_extras_requirements("all", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_ci", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_all", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("all_dbs", ALL_DB_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_hadoop", ["apache.hdfs", "apache.hive", "presto"])
+
+
+class Develop(develop_orig):
+    """Forces removal of providers in editable mode."""
+
+    def run(self):
+        self.announce('Installing in editable mode. Uninstalling provider packages!', level=log.INFO)
+        # We need to run "python3 -m pip" because it might be that older PIP binary is in the path
+        # And it results with an error when running pip directly (cannot import pip module)
+        # also PIP does not have a stable API so we have to run subprocesses ¯\_(ツ)_/¯
+        try:
+            installed_packages = (
+                subprocess.check_output(["python3", "-m", "pip", "freeze"]).decode().splitlines()
+            )
+            airflow_provider_packages = [
+                package_line.split("=")[0]
+                for package_line in installed_packages
+                if package_line.startswith("apache-airflow-providers")
+            ]
+            self.announce(f'Uninstalling ${airflow_provider_packages}!', level=log.INFO)
+            subprocess.check_call(["python3", "-m", "pip", "uninstall", "--yes", *airflow_provider_packages])
+        except subprocess.CalledProcessError as e:
+            self.announce(f'Error when uninstalling airflow provider packages: {e}!', level=log.WARN)
+        super().run()
+
+
+class Install(install_orig):
+    """Forces installation of providers from sources in editable mode."""
 
+    def run(self):
+        self.announce('Standard installation. Providers are installed from packages', level=log.INFO)

Review comment:
       Actually it shows up even with one -v, but the output of -v is fantastically (or rather wanted to say fanatically) verbose in this case, so it does not really matter. This is about 5% of the log printed with -v :)
   
   ```
   copying airflow/www/templates/airflow/variable_edit.html -> build/lib/airflow/www/templates/airflow
     copying airflow/www/templates/airflow/variable_list.html -> build/lib/airflow/www/templates/airflow
     copying airflow/www/templates/airflow/xcom.html -> build/lib/airflow/www/templates/airflow
     creating build/lib/airflow/www/templates/analytics
     copying airflow/www/templates/analytics/google_analytics.html -> build/lib/airflow/www/templates/analytics
     copying airflow/www/templates/analytics/metarouter.html -> build/lib/airflow/www/templates/analytics
     copying airflow/www/templates/analytics/segment.html -> build/lib/airflow/www/templates/analytics
     creating build/lib/airflow/www/templates/appbuilder
     copying airflow/www/templates/appbuilder/custom_icons.html -> build/lib/airflow/www/templates/appbuilder
     copying airflow/www/templates/appbuilder/dag_docs.html -> build/lib/airflow/www/templates/appbuilder
     copying airflow/www/templates/appbuilder/flash.html -> build/lib/airflow/www/templates/appbuilder
     copying airflow/www/templates/appbuilder/index.html -> build/lib/airflow/www/templates/appbuilder
     copying airflow/www/templates/appbuilder/loading_dots.html -> build/lib/airflow/www/templates/appbuilder
     copying airflow/www/templates/appbuilder/navbar.html -> build/lib/airflow/www/templates/appbuilder
     copying airflow/www/templates/appbuilder/navbar_menu.html -> build/lib/airflow/www/templates/appbuilder
     copying airflow/www/templates/appbuilder/navbar_right.html -> build/lib/airflow/www/templates/appbuilder
     creating build/lib/airflow/api_connexion/openapi
     copying airflow/api_connexion/openapi/v1.yaml -> build/lib/airflow/api_connexion/openapi
     installing to build/bdist.linux-x86_64/wheel
     running install
     Standard installation. Providers are installed from packages
     running install_lib
     creating build/bdist.linux-x86_64
     creating build/bdist.linux-x86_64/wheel
     creating build/bdist.linux-x86_64/wheel/airflow
     creating build/bdist.linux-x86_64/wheel/airflow/serialization
     copying build/lib/airflow/serialization/schema.json -> build/bdist.linux-x86_64/wheel/airflow/serialization
     copying build/lib/airflow/serialization/json_schema.py -> build/bdist.linux-x86_64/wheel/airflow/serialization
     copying build/lib/airflow/serialization/helpers.py -> build/bdist.linux-x86_64/wheel/airflow/serialization
     copying build/lib/airflow/serialization/enums.py -> build/bdist.linux-x86_64/wheel/airflow/serialization
     copying build/lib/airflow/serialization/serialized_objects.py -> build/bdist.linux-x86_64/wheel/airflow/serialization
     copying build/lib/airflow/serialization/__init__.py -> build/bdist.linux-x86_64/wheel/airflow/serialization
     copying build/lib/airflow/customized_form_field_behaviours.schema.json -> build/bdist.linux-x86_64/wheel/airflow
     creating build/bdist.linux-x86_64/wheel/airflow/macros
     copying build/lib/airflow/macros/hive.py -> build/bdist.linux-x86_64/wheel/airflow/macros
     copying build/lib/airflow/macros/__init__.py -> build/bdist.linux-x86_64/wheel/airflow/macros
     creating build/bdist.linux-x86_64/wheel/airflow/mypy
     creating build/bdist.linux-x86_64/wheel/airflow/mypy/plugin
     copying build/lib/airflow/mypy/plugin/decorators.py -> build/bdist.linux-x86_64/wheel/airflow/mypy/plugin
   
   ```




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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


   [The Workflow run](https://github.com/apache/airflow/actions/runs/458253668) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on a change in pull request #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -640,15 +672,21 @@ def find_requirements_for_alias(alias_to_look_for: Tuple[str, str]) -> List[str]
         raise Exception(f"The extra {new_extra} is missing for alias {deprecated_extra}")
 
 
-# Add extras for all deprecated aliases. Requirements for those deprecated aliases are the same
-# as the extras they are replaced with
-for alias, extra in EXTRAS_DEPRECATED_ALIASES.items():
-    requirements = EXTRAS_REQUIREMENTS.get(extra) if extra != '' else []
-    if requirements is None:
-        raise Exception(f"The extra {extra} is missing for deprecated alias {alias}")
-    # Note the requirements are not copies - those are the same lists as for the new extras. This is intended.
-    # Thanks to that if the original extras are later extended with providers, aliases are extended as well.
-    EXTRAS_REQUIREMENTS[alias] = requirements
+def add_extras_for_all_deprecated_aliases() -> None:
+    """
+    Add extras for all deprecated aliases. Requirements for those deprecated aliases are the same
+    as the extras they are replaced with.
+    The requirements are not copies - those are the same lists as for the new extras. This is intended.
+    Thanks to that if the original extras are later extended with providers, aliases are extended as well.
+    """
+    for alias, extra in EXTRAS_DEPRECATED_ALIASES.items():
+        requirements = EXTRAS_REQUIREMENTS.get(extra) if extra != '' else []
+        if requirements is None:
+            raise Exception(f"The extra {extra} is missing for deprecated alias {alias}")
+        EXTRAS_REQUIREMENTS[alias] = requirements
+
+
+add_extras_for_all_deprecated_aliases()

Review comment:
       I looked at it and yeah, we pretty much always want them called. The main reason I added the methods is to make them named/pydoced and also it makes it better to reuse some of the common variable names (extras for onw) because if they are in the "top" code, then they can easily hide other variable rather than be 'local' in functions. So less warnings from PyCharm for one.

##########
File path: setup.py
##########
@@ -640,15 +672,21 @@ def find_requirements_for_alias(alias_to_look_for: Tuple[str, str]) -> List[str]
         raise Exception(f"The extra {new_extra} is missing for alias {deprecated_extra}")
 
 
-# Add extras for all deprecated aliases. Requirements for those deprecated aliases are the same
-# as the extras they are replaced with
-for alias, extra in EXTRAS_DEPRECATED_ALIASES.items():
-    requirements = EXTRAS_REQUIREMENTS.get(extra) if extra != '' else []
-    if requirements is None:
-        raise Exception(f"The extra {extra} is missing for deprecated alias {alias}")
-    # Note the requirements are not copies - those are the same lists as for the new extras. This is intended.
-    # Thanks to that if the original extras are later extended with providers, aliases are extended as well.
-    EXTRAS_REQUIREMENTS[alias] = requirements
+def add_extras_for_all_deprecated_aliases() -> None:
+    """
+    Add extras for all deprecated aliases. Requirements for those deprecated aliases are the same
+    as the extras they are replaced with.
+    The requirements are not copies - those are the same lists as for the new extras. This is intended.
+    Thanks to that if the original extras are later extended with providers, aliases are extended as well.
+    """
+    for alias, extra in EXTRAS_DEPRECATED_ALIASES.items():
+        requirements = EXTRAS_REQUIREMENTS.get(extra) if extra != '' else []
+        if requirements is None:
+            raise Exception(f"The extra {extra} is missing for deprecated alias {alias}")
+        EXTRAS_REQUIREMENTS[alias] = requirements
+
+
+add_extras_for_all_deprecated_aliases()

Review comment:
       I looked at it and yeah, we pretty much always want them called. The main reason I added the methods is to make them named/pydoced and also it makes it better to reuse some of the common variable names (extras for one) because if they are in the "top" code, then they can easily hide other variable rather than be 'local' in functions. So less warnings from PyCharm for one.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on a change in pull request #13439: Forces unistalling providers in editable mode.

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



##########
File path: INSTALL
##########
@@ -51,24 +51,35 @@ pip install . \
   --constraint "https://raw.githubusercontent.com/apache/airflow/constraints-master/constraints-3.6.txt"
 
 
-By default `pip install` in Airflow 2.0 installs only the provider packages that are needed by the extras,
-however if you want to install all providers (which was default behaviour in 1.10.*)
-you can do it by setting environment variable INSTALL_PROVIDERS_FROM_SOURCES to `true`.
+By default `pip install` in Airflow 2.0 installs only the provider packages that are needed by the extras and
+install them as packages from PyPI rather than from local sources:
 
 
-INSTALL_PROVIDERS_FROM_SOURCES="true" pip install . \
+pip install . \
   --constraint "https://raw.githubusercontent.com/apache/airflow/constraints-master/constraints-3.6.txt"
 
 
-You can also install airflow in "editable mode" (with -e) flag and then provider packages will be
-available, because they are used directly from the airflow sources:
-
+You can also install airflow in "editable mode" (with -e) flag and then provider packages are
+available directly from the sources (and the provider packages installed from PyPI are uninstalled in
+order to avoid having providers in two places. This is useful if you want to develop providers:
 
 pip install -e . \
   --constraint "https://raw.githubusercontent.com/apache/airflow/constraints-master/constraints-3.6.txt"
 
+You can als skip installing provider packages from PyPI by setting INSTALL_PROVIDERS_FROM_SOURCE to "true".
+In this case Airflow will be installed in non-editable mode with all providers installed from the sources.
+Additionally provider.yaml files will also be copied to providers folders which will make the providers

Review comment:
       Good point with the description of provider.yaml file. I added a chapter about it in the 'Provider Packages' documentation  and referred to 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 #13439: Forces unistalling providers in editable mode.

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



##########
File path: INSTALL
##########
@@ -51,24 +51,41 @@ pip install . \
   --constraint "https://raw.githubusercontent.com/apache/airflow/constraints-master/constraints-3.6.txt"
 
 
-By default `pip install` in Airflow 2.0 installs only the provider packages that are needed by the extras,
-however if you want to install all providers (which was default behaviour in 1.10.*)
-you can do it by setting environment variable INSTALL_PROVIDERS_FROM_SOURCES to `true`.
+By default `pip install` in Airflow 2.0 installs only the provider packages that are needed by the extras and
+install them as packages from PyPI rather than from local sources:
 
 
-INSTALL_PROVIDERS_FROM_SOURCES="true" pip install . \
+pip install . \
   --constraint "https://raw.githubusercontent.com/apache/airflow/constraints-master/constraints-3.6.txt"
 
 
-You can also install airflow in "editable mode" (with -e) flag and then provider packages will be
-available, because they are used directly from the airflow sources:
+You can also install airflow in "editable mode" (with -e) flag and then provider packages are
+available directly from the sources (and the provider packages installed from PyPI are UNINSTALLED in
+order to avoid having providers in two places. And `provider.yaml` files are used to discover capabilities
+of the providers which are part of the airflow source code.
+
+You can read more about `provider.yaml` and community-managed providers in
+https://airflow.apache.org/docs/apache-airflow-providers/index.html
 
+This is useful if you want to develop providers:
 
 pip install -e . \
   --constraint "https://raw.githubusercontent.com/apache/airflow/constraints-master/constraints-3.6.txt"
 
+You can als skip installing provider packages from PyPI by setting INSTALL_PROVIDERS_FROM_SOURCE to "true".
+In this case Airflow will be installed in non-editable mode with all providers installed from the sources.
+Additionally `provider.yaml` files will also be copied to providers folders which will make the providers
+discoverable by Airflow even if they are not installed from packages in this case.
+
+INSTALL_PROVIDERS_FROM_SOURCES="true" pip install . \
+  --constraint "https://raw.githubusercontent.com/apache/airflow/constraints-master/constraints-3.6.txt"
+
+Airflow can be installed with extras to install some additional features (for example 'async' or 'doc' or
+to install automatically providers and all dependencies needed by that provider:
+
+pip install .[async,google,amazon] \
+  --constraint "https://raw.githubusercontent.com/apache/airflow/constraints-master/constraints-3.6.txt"
 
-# You can also install Airflow with extras specified. The list of available extras:

Review comment:
       Worth having the following line:
   
   ```
   The list of available extras:
   ```




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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


   [The Workflow run](https://github.com/apache/airflow/actions/runs/461932592) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on a change in pull request #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -150,13 +162,22 @@ def write_version(filename: str = os.path.join(*[my_dir, "airflow", "git_version
         file.write(text)
 
 
-if os.environ.get('USE_THEME_FROM_GIT'):
-    _SPHINX_AIRFLOW_THEME_URL = (
-        "@ https://github.com/apache/airflow-site/releases/download/0.0.4/"
-        "sphinx_airflow_theme-0.0.4-py3-none-any.whl"
-    )
-else:
-    _SPHINX_AIRFLOW_THEME_URL = ''
+def get_sphinx_theme_version() -> str:
+    """
+    Return sphinx theme version. If USE_THEME_FROM_GIT env variable is set, the theme is used from
+    github tp allow dynamically update it during development. However for regular PIP release
+    you cannot use @ package specification, so the latest available released theme package from
+    PIP is used.
+    :return: the
+    """
+    if os.environ.get('USE_THEME_FROM_GIT'):
+        return (
+            "@ https://github.com/apache/airflow-site/releases/download/0.0.4/"
+            + "sphinx_airflow_theme-0.0.4-py3-none-any.whl"
+        )
+    else:
+        return ''

Review comment:
       True.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] github-actions[bot] commented on pull request #13439: Forces unistalling providers in editable mode.

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


   [The Workflow run](https://github.com/apache/airflow/actions/runs/457875265) is cancelling this PR. Building image for the PR has been cancelled


----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -150,13 +162,22 @@ def write_version(filename: str = os.path.join(*[my_dir, "airflow", "git_version
         file.write(text)
 
 
-if os.environ.get('USE_THEME_FROM_GIT'):
-    _SPHINX_AIRFLOW_THEME_URL = (
-        "@ https://github.com/apache/airflow-site/releases/download/0.0.4/"
-        "sphinx_airflow_theme-0.0.4-py3-none-any.whl"
-    )
-else:
-    _SPHINX_AIRFLOW_THEME_URL = ''
+def get_sphinx_theme_version() -> str:
+    """
+    Return sphinx theme version. If USE_THEME_FROM_GIT env variable is set, the theme is used from
+    github tp allow dynamically update it during development. However for regular PIP release
+    you cannot use @ package specification, so the latest available released theme package from
+    PIP is used.
+    :return: the
+    """
+    if os.environ.get('USE_THEME_FROM_GIT'):
+        return (
+            "@ https://github.com/apache/airflow-site/releases/download/0.0.4/"
+            + "sphinx_airflow_theme-0.0.4-py3-none-any.whl"
+        )
+    else:
+        return ''

Review comment:
       Also removed same with git_version above.




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -597,10 +618,21 @@ def write_version(filename: str = os.path.join(*[my_dir, "airflow", "git_version
     'virtualenv': virtualenv,
 }
 
-# Add extras for all providers. For all providers the extras name = providers name
-for provider_name, provider_requirement in PROVIDERS_REQUIREMENTS.items():
-    EXTRAS_REQUIREMENTS[provider_name] = provider_requirement
+
+def add_extras_for_all_providers() -> None:
+    """
+    Adds extras for all providers.
+    By default all providers have the same extra name as provider id, for example
+    'apache.hive' extra has 'apache.hive' provider requirement.
+    :return:

Review comment:
       ```suggestion
   ```
   
   Since it is empty it is better to 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



[GitHub] [airflow] potiuk commented on a change in pull request #13439: Forces unistalling providers in editable mode.

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



##########
File path: LOCAL_VIRTUALENV.rst
##########
@@ -135,17 +134,40 @@ To create and initialize the local virtualenv:
 
    .. code-block:: bash
 
-    pip install -U -e ".[devel,<OTHER EXTRAS>]" # for example: pip install -U -e ".[devel,google,postgres]"
+    pip install --upgrade -e ".[devel,<OTHER EXTRAS>]" # for example: pip install --upgrade -e ".[devel,google,postgres]"
 
 In case you have problems with installing airflow because of some requirements are not installable, you can
 try to install it with the set of working constraints (note that there are different constraint files
 for different python versions:
 
    .. code-block:: bash
 
-    pip install -U -e ".[devel,<OTHER EXTRAS>]" \
+    pip install -e ".[devel,<OTHER EXTRAS>]" \
+        --constraint "https://raw.githubusercontent.com/apache/airflow/constraints-master/constraints-3.6.txt"
+
+
+This will install Airflow in 'editable' mode - where sources of Airflow are taken directly from the source
+code rather than moved to the installation directory. During the installation airflow will install - but then
+automatically remove all provider packages installed from pypi - instead it will automatically use the
+provider packages available in your local sources.

Review comment:
       Will do.




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -640,15 +672,21 @@ def find_requirements_for_alias(alias_to_look_for: Tuple[str, str]) -> List[str]
         raise Exception(f"The extra {new_extra} is missing for alias {deprecated_extra}")
 
 
-# Add extras for all deprecated aliases. Requirements for those deprecated aliases are the same
-# as the extras they are replaced with
-for alias, extra in EXTRAS_DEPRECATED_ALIASES.items():
-    requirements = EXTRAS_REQUIREMENTS.get(extra) if extra != '' else []
-    if requirements is None:
-        raise Exception(f"The extra {extra} is missing for deprecated alias {alias}")
-    # Note the requirements are not copies - those are the same lists as for the new extras. This is intended.
-    # Thanks to that if the original extras are later extended with providers, aliases are extended as well.
-    EXTRAS_REQUIREMENTS[alias] = requirements
+def add_extras_for_all_deprecated_aliases() -> None:
+    """
+    Add extras for all deprecated aliases. Requirements for those deprecated aliases are the same
+    as the extras they are replaced with.
+    The requirements are not copies - those are the same lists as for the new extras. This is intended.
+    Thanks to that if the original extras are later extended with providers, aliases are extended as well.
+    """
+    for alias, extra in EXTRAS_DEPRECATED_ALIASES.items():
+        requirements = EXTRAS_REQUIREMENTS.get(extra) if extra != '' else []
+        if requirements is None:
+            raise Exception(f"The extra {extra} is missing for deprecated alias {alias}")
+        EXTRAS_REQUIREMENTS[alias] = requirements
+
+
+add_extras_for_all_deprecated_aliases()

Review comment:
       The only reason I made it here is to be able to remove the whole section for "deprecated aliases. But I think I can move them to main indeed. In pre-commit where I check whether setup.py and documentation are synchronized, I already call the one function which I call in main. There might be some dependencies between the sequence of calls and definitions of dicts/arrays, but I think I can solve all of them. Let me see.




----------------------------------------------------------------
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] turbaszek commented on a change in pull request #13439: Forces unistalling providers in editable mode.

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



##########
File path: LOCAL_VIRTUALENV.rst
##########
@@ -135,17 +134,40 @@ To create and initialize the local virtualenv:
 
    .. code-block:: bash
 
-    pip install -U -e ".[devel,<OTHER EXTRAS>]" # for example: pip install -U -e ".[devel,google,postgres]"
+    pip install --upgrade -e ".[devel,<OTHER EXTRAS>]" # for example: pip install --upgrade -e ".[devel,google,postgres]"
 
 In case you have problems with installing airflow because of some requirements are not installable, you can
 try to install it with the set of working constraints (note that there are different constraint files
 for different python versions:
 
    .. code-block:: bash
 
-    pip install -U -e ".[devel,<OTHER EXTRAS>]" \
+    pip install -e ".[devel,<OTHER EXTRAS>]" \
+        --constraint "https://raw.githubusercontent.com/apache/airflow/constraints-master/constraints-3.6.txt"
+
+
+This will install Airflow in 'editable' mode - where sources of Airflow are taken directly from the source
+code rather than moved to the installation directory. During the installation airflow will install - but then
+automatically remove all provider packages installed from pypi - instead it will automatically use the
+provider packages available in your local sources.
+
+You can also install Airflow in non-editable mode:
+
+   .. code-block:: bash
+
+    pip install ".[devel,<OTHER EXTRAS>]" \
         --constraint "https://raw.githubusercontent.com/apache/airflow/constraints-master/constraints-3.6.txt"
 
+This will copy the sources to directory where usually python packages are installed. Yyou can see the list

Review comment:
       ```suggestion
   This will copy the sources to directory where usually python packages are installed. You can see the list
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on a change in pull request #13439: Forces unistalling providers in editable mode.

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



##########
File path: docs/apache-airflow-providers/index.rst
##########
@@ -87,9 +88,24 @@ The capabilities are:
   connections defined by the provider. See :doc:`apache-airflow:howto/connection` for a description of
   connection and what capabilities of custom connection you can define.
 
+Custom provider packages
+''''''''''''''''''''''''
+
+However, there is more. You can develop your own providers. This is a bit involved, but your custom operators,
+hooks, sensors, transfer operators can be packaged together in a standard airflow package and installed
+using the same mechanisms. Moreover they can also use the same mechanisms to extend the Airflow Core with
+custom connections and extra operator links as described in the previous chapter. See
+[Developing custom providers](https://github.com/apache/airflow/blob/master/DEVELOPING_CUSTOM_PROVIDERS.rst)

Review comment:
       I updated the documentation a bit, adding more requirements for custom provider documentation, mentioning testing/documentaion requirements. 




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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



##########
File path: CONTRIBUTING.rst
##########
@@ -712,6 +712,44 @@ snowflake                  slack
 
   .. END PACKAGE DEPENDENCIES HERE
 
+
+Developing provider packages
+----------------------------
+
+While you can develop your own providers, Apache Airflow has 60+ providers that are managed by the community.
+They are part of the same repository as Apache Airflow (we use ``monorepo`` approach where different
+parts of the system are developed in the same repository but then they are packaged and released separately).
+All the community-managed providers are in 'airflow/providers' folder and they are all sub-packages of
+'airflow.providers' package. All the providers are available as ``apache-airflow-providers-<PROVIDER_ID>``
+packages.
+
+The capabilities of the community-managed providers are the same as the third-party ones are the same. When
+the providers are installed from PyPI, they provide the entry-point containing the metadata as described
+in the previous chapter. However when they are locally developed, together with Airflow, the mechanism
+of discovery of the providers is based on ``provider.yaml`` file the is placed in the top-folder of
+the provider. Similarly as in case of the ``The ``provider.yaml`` file is compliant with the

Review comment:
       ```suggestion
   the provider. Similarly as in case of the ``provider.yaml`` file is compliant with the
   ```




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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



##########
File path: docs/apache-airflow-providers/index.rst
##########
@@ -87,9 +88,24 @@ The capabilities are:
   connections defined by the provider. See :doc:`apache-airflow:howto/connection` for a description of
   connection and what capabilities of custom connection you can define.
 
+Custom provider packages
+''''''''''''''''''''''''
+
+However, there is more. You can develop your own providers. This is a bit involved, but your custom operators,
+hooks, sensors, transfer operators can be packaged together in a standard airflow package and installed
+using the same mechanisms. Moreover they can also use the same mechanisms to extend the Airflow Core with
+custom connections and extra operator links as described in the previous chapter. See
+[Developing custom providers](https://github.com/apache/airflow/blob/master/DEVELOPING_CUSTOM_PROVIDERS.rst)

Review comment:
       where is this file? -- I think currently it is in a separate section in 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



[GitHub] [airflow] potiuk commented on pull request #13439: Forces unistalling providers in editable mode.

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


   This one is green. Looking forward to approvals!


----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -780,76 +826,135 @@ def get_provider_package_from_package_id(package_id: str):
 
 
 class AirflowDistribution(Distribution):
-    """setuptools.Distribution subclass with Airflow specific behaviour"""
+    """
+    The setuptools.Distribution subclass with Airflow specific behaviour
+
+    The reason for pylint: disable=signature-differs of parse_config_files is explained here:
+    https://github.com/PyCQA/pylint/issues/3737
+
+    """
 
-    # https://github.com/PyCQA/pylint/issues/3737
     def parse_config_files(self, *args, **kwargs):  # pylint: disable=signature-differs
         """
         Ensure that when we have been asked to install providers from sources
-        that we don't *also* try to install those providers from PyPI
+        that we don't *also* try to install those providers from PyPI.
+        Also we should make sure that in this case we copy provider.yaml files so that
+        Providers manager can find package information.
         """
         super().parse_config_files(*args, **kwargs)
-        if os.getenv('INSTALL_PROVIDERS_FROM_SOURCES') == 'true':
+        if os.getenv(INSTALL_PROVIDERS_FROM_SOURCES) == 'true':
             self.install_requires = [  # noqa  pylint: disable=attribute-defined-outside-init
                 req for req in self.install_requires if not req.startswith('apache-airflow-providers-')
             ]
+            provider_yaml_files = glob.glob("airflow/providers/**/provider.yaml", recursive=True)
+            for provider_yaml_file in provider_yaml_files:
+                provider_relative_path = relpath(provider_yaml_file, os.path.join(my_dir, "airflow"))
+                self.package_data['airflow'].append(provider_relative_path)
         else:
             self.install_requires.extend(
                 [get_provider_package_from_package_id(package_id) for package_id in PREINSTALLED_PROVIDERS]
             )
 
 
-def add_provider_packages_to_requirements(extra_with_providers: str, providers: List[str]):
+def add_provider_packages_to_extras_requirements(extra: str, providers: List[str]) -> None:
     """
-    Adds provider packages to requirements
+    Adds provider packages to requirements of extra.
 
-    :param extra_with_providers: Name of the extra to add providers to
-    :param providers: list of provider names
+    :param extra: Name of the extra to add providers to
+    :param providers: list of provider ids
     """
-    EXTRAS_WITH_PROVIDERS.add(extra_with_providers)
-    EXTRAS_REQUIREMENTS[extra_with_providers].extend(
+    EXTRAS_WITH_PROVIDERS.add(extra)
+    EXTRAS_REQUIREMENTS[extra].extend(
         [get_provider_package_from_package_id(package_name) for package_name in providers]
     )
 
 
-def add_all_provider_packages():
+def add_all_provider_packages() -> None:
     """
-    In case of regular installation (when INSTALL_PROVIDERS_FROM_SOURCES is false), we should
-    add extra dependencies to Airflow - to get the providers automatically installed when
-    those extras are installed.
+    In case of regular installation (providers installed from packages), we should add extra dependencies to
+    Airflow - to get the providers automatically installed when those extras are installed.
+
+    For providers installed from sources we skip that step. That helps to test and install airflow with
+    all packages in CI - for example when new providers are added, otherwise the installation would fail
+    as the new provider is not yet in PyPI.
 
     """
     for provider in ALL_PROVIDERS:
-        add_provider_packages_to_requirements(provider, [provider])
-    add_provider_packages_to_requirements("all", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("devel_ci", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("devel_all", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("all_dbs", ALL_DB_PROVIDERS)
-    add_provider_packages_to_requirements("devel_hadoop", ["apache.hdfs", "apache.hive", "presto"])
+        add_provider_packages_to_extras_requirements(provider, [provider])
+    add_provider_packages_to_extras_requirements("all", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_ci", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_all", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("all_dbs", ALL_DB_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_hadoop", ["apache.hdfs", "apache.hive", "presto"])
+
+
+class Develop(develop_orig):
+    """Forces removal of providers in editable mode."""
+
+    def run(self):
+        self.announce('Installing in editable mode. Uninstalling provider packages!', level=log.INFO)
+        # We need to run "python3 -m pip" because it might be that older PIP binary is in the path
+        # And it results with an error when running pip directly (cannot import pip module)
+        # also PIP does not have a stable API so we have to run subprocesses ¯\_(ツ)_/¯
+        try:
+            installed_packages = (
+                subprocess.check_output(["python3", "-m", "pip", "freeze"]).decode().splitlines()
+            )
+            airflow_provider_packages = [
+                package_line.split("=")[0]
+                for package_line in installed_packages
+                if package_line.startswith("apache-airflow-providers")
+            ]
+            self.announce(f'Uninstalling ${airflow_provider_packages}!', level=log.INFO)
+            subprocess.check_call(["python3", "-m", "pip", "uninstall", "--yes", *airflow_provider_packages])
+        except subprocess.CalledProcessError as e:
+            self.announce(f'Error when uninstalling airflow provider packages: {e}!', level=log.WARN)
+        super().run()
+
+
+class Install(install_orig):
+    """Forces installation of providers from sources in editable mode."""
 
+    def run(self):
+        self.announce('Standard installation. Providers are installed from packages', level=log.INFO)
+        super().run()
 
-def do_setup():
-    """Perform the Airflow package setup."""
+
+def do_setup() -> None:
+    """
+    Perform the Airflow package setup.
+    Most values come from setup.cfg, only the dynamically calculated ones are passed to setup
+    function call. See https://setuptools.readthedocs.io/en/latest/userguide/declarative_config.html
+    """
     setup_kwargs = {}
 
-    if os.getenv('INSTALL_PROVIDERS_FROM_SOURCES') == 'true':
-        # Only specify this if we need this option, otherwise let default from
-        # setup.cfg control this (kwargs in setup() call take priority)
-        setup_kwargs['packages'] = find_namespace_packages(include=['airflow*'])
+    def include_provider_namespace_packages_when_installing_from_sources() -> None:

Review comment:
       The main reason was that I wanted to  name it appropriately. I think everywhere in a code (especially in such case where you do not care about a performance), if you have a need to write a paragraph of comment, it's much better to replace it with a function that have the comment converted to pydoc, and is named appropriately, even if the code is short (but not obvious why we are doing it).
   
   This way when you look at the code you can focus on what the function does (fucntion name), why we do it (pydoc when for example hovering your mouse over the function) and how it does it (function body) separately and understand it at different levels. 




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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



##########
File path: docs/apache-airflow-providers/index.rst
##########
@@ -119,6 +132,43 @@ When you write your own provider, consider following the
 `Naming conventions for provider packages <https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#naming-conventions-for-provider-packages>`_
 
 
+Community-managed vs. 3rd party providers
+'''''''''''''''''''''''''''''''''''''''''
+
+While you can develop your own providers, Apache Airflow has 60+ providers that are managed by the community.
+They are part of the same repository as Apache Airflow (we use ``monorepo`` approach where different
+parts of the system are developed in the same repository but then they are packaged and released separately).
+All the community-managed providers are in 'airflow/providers' folder and they are all sub-packages of
+'airflow.providers' package. All the providers are available as ``apache-airflow-providers-<PROVIDER_ID>``
+packages.
+
+The capabilities of the community-managed providers are the same as the third-party ones are the same. When
+the providers are installed from PyPI, they provide the entry-point containing the metadata as described
+in the previous chapter. However when they are locally developed, together with Airflow, the mechanism

Review comment:
       Actuallly moved this content to 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



[GitHub] [airflow] potiuk commented on pull request #13439: Forces unistalling providers in editable mode.

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


   All comments addressed !


----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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



##########
File path: CONTRIBUTING.rst
##########
@@ -712,6 +712,44 @@ snowflake                  slack
 
   .. END PACKAGE DEPENDENCIES HERE
 
+
+Developing provider packages
+----------------------------
+
+While you can develop your own providers, Apache Airflow has 60+ providers that are managed by the community.
+They are part of the same repository as Apache Airflow (we use ``monorepo`` approach where different
+parts of the system are developed in the same repository but then they are packaged and released separately).
+All the community-managed providers are in 'airflow/providers' folder and they are all sub-packages of
+'airflow.providers' package. All the providers are available as ``apache-airflow-providers-<PROVIDER_ID>``
+packages.
+
+The capabilities of the community-managed providers are the same as the third-party ones are the same. When
+the providers are installed from PyPI, they provide the entry-point containing the metadata as described
+in the previous chapter. However when they are locally developed, together with Airflow, the mechanism
+of discovery of the providers is based on ``provider.yaml`` file the is placed in the top-folder of

Review comment:
       ```suggestion
   of the discovery of the providers is based on ``provider.yaml`` file that is placed in the top-folder of
   ```




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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



##########
File path: docs/apache-airflow-providers/index.rst
##########
@@ -87,9 +88,24 @@ The capabilities are:
   connections defined by the provider. See :doc:`apache-airflow:howto/connection` for a description of
   connection and what capabilities of custom connection you can define.
 
+Custom provider packages
+''''''''''''''''''''''''
+
+However, there is more. You can develop your own providers. This is a bit involved, but your custom operators,
+hooks, sensors, transfer operators can be packaged together in a standard airflow package and installed
+using the same mechanisms. Moreover they can also use the same mechanisms to extend the Airflow Core with
+custom connections and extra operator links as described in the previous chapter. See
+[Developing custom providers](https://github.com/apache/airflow/blob/master/DEVELOPING_CUSTOM_PROVIDERS.rst)

Review comment:
       Actually I remove it. Because it is described in the next chapter. This is user-facing documentation if you want to develop your own, custom provider (with a nice FAQ entry below added by one of the users who already developed their own provider and share the learnings). The CONTRIBUTING doc is only for those who would like to develop the community provider (with documentation, provider.yaml etc).




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -780,76 +826,135 @@ def get_provider_package_from_package_id(package_id: str):
 
 
 class AirflowDistribution(Distribution):
-    """setuptools.Distribution subclass with Airflow specific behaviour"""
+    """
+    The setuptools.Distribution subclass with Airflow specific behaviour
+
+    The reason for pylint: disable=signature-differs of parse_config_files is explained here:
+    https://github.com/PyCQA/pylint/issues/3737
+
+    """
 
-    # https://github.com/PyCQA/pylint/issues/3737
     def parse_config_files(self, *args, **kwargs):  # pylint: disable=signature-differs
         """
         Ensure that when we have been asked to install providers from sources
-        that we don't *also* try to install those providers from PyPI
+        that we don't *also* try to install those providers from PyPI.
+        Also we should make sure that in this case we copy provider.yaml files so that
+        Providers manager can find package information.
         """
         super().parse_config_files(*args, **kwargs)
-        if os.getenv('INSTALL_PROVIDERS_FROM_SOURCES') == 'true':
+        if os.getenv(INSTALL_PROVIDERS_FROM_SOURCES) == 'true':
             self.install_requires = [  # noqa  pylint: disable=attribute-defined-outside-init
                 req for req in self.install_requires if not req.startswith('apache-airflow-providers-')
             ]
+            provider_yaml_files = glob.glob("airflow/providers/**/provider.yaml", recursive=True)
+            for provider_yaml_file in provider_yaml_files:
+                provider_relative_path = relpath(provider_yaml_file, os.path.join(my_dir, "airflow"))
+                self.package_data['airflow'].append(provider_relative_path)
         else:
             self.install_requires.extend(
                 [get_provider_package_from_package_id(package_id) for package_id in PREINSTALLED_PROVIDERS]
             )
 
 
-def add_provider_packages_to_requirements(extra_with_providers: str, providers: List[str]):
+def add_provider_packages_to_extras_requirements(extra: str, providers: List[str]) -> None:
     """
-    Adds provider packages to requirements
+    Adds provider packages to requirements of extra.
 
-    :param extra_with_providers: Name of the extra to add providers to
-    :param providers: list of provider names
+    :param extra: Name of the extra to add providers to
+    :param providers: list of provider ids
     """
-    EXTRAS_WITH_PROVIDERS.add(extra_with_providers)
-    EXTRAS_REQUIREMENTS[extra_with_providers].extend(
+    EXTRAS_WITH_PROVIDERS.add(extra)
+    EXTRAS_REQUIREMENTS[extra].extend(
         [get_provider_package_from_package_id(package_name) for package_name in providers]
     )
 
 
-def add_all_provider_packages():
+def add_all_provider_packages() -> None:
     """
-    In case of regular installation (when INSTALL_PROVIDERS_FROM_SOURCES is false), we should
-    add extra dependencies to Airflow - to get the providers automatically installed when
-    those extras are installed.
+    In case of regular installation (providers installed from packages), we should add extra dependencies to
+    Airflow - to get the providers automatically installed when those extras are installed.
+
+    For providers installed from sources we skip that step. That helps to test and install airflow with
+    all packages in CI - for example when new providers are added, otherwise the installation would fail
+    as the new provider is not yet in PyPI.
 
     """
     for provider in ALL_PROVIDERS:
-        add_provider_packages_to_requirements(provider, [provider])
-    add_provider_packages_to_requirements("all", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("devel_ci", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("devel_all", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("all_dbs", ALL_DB_PROVIDERS)
-    add_provider_packages_to_requirements("devel_hadoop", ["apache.hdfs", "apache.hive", "presto"])
+        add_provider_packages_to_extras_requirements(provider, [provider])
+    add_provider_packages_to_extras_requirements("all", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_ci", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_all", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("all_dbs", ALL_DB_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_hadoop", ["apache.hdfs", "apache.hive", "presto"])
+
+
+class Develop(develop_orig):
+    """Forces removal of providers in editable mode."""
+
+    def run(self):
+        self.announce('Installing in editable mode. Uninstalling provider packages!', level=log.INFO)
+        # We need to run "python3 -m pip" because it might be that older PIP binary is in the path
+        # And it results with an error when running pip directly (cannot import pip module)
+        # also PIP does not have a stable API so we have to run subprocesses ¯\_(ツ)_/¯
+        try:
+            installed_packages = (
+                subprocess.check_output(["python3", "-m", "pip", "freeze"]).decode().splitlines()
+            )
+            airflow_provider_packages = [
+                package_line.split("=")[0]
+                for package_line in installed_packages
+                if package_line.startswith("apache-airflow-providers")
+            ]
+            self.announce(f'Uninstalling ${airflow_provider_packages}!', level=log.INFO)
+            subprocess.check_call(["python3", "-m", "pip", "uninstall", "--yes", *airflow_provider_packages])
+        except subprocess.CalledProcessError as e:
+            self.announce(f'Error when uninstalling airflow provider packages: {e}!', level=log.WARN)
+        super().run()
+
+
+class Install(install_orig):
+    """Forces installation of providers from sources in editable mode."""
 
+    def run(self):
+        self.announce('Standard installation. Providers are installed from packages', level=log.INFO)
+        super().run()
 
-def do_setup():
-    """Perform the Airflow package setup."""
+
+def do_setup() -> None:
+    """
+    Perform the Airflow package setup.
+    Most values come from setup.cfg, only the dynamically calculated ones are passed to setup
+    function call. See https://setuptools.readthedocs.io/en/latest/userguide/declarative_config.html
+    """
     setup_kwargs = {}
 
-    if os.getenv('INSTALL_PROVIDERS_FROM_SOURCES') == 'true':
-        # Only specify this if we need this option, otherwise let default from
-        # setup.cfg control this (kwargs in setup() call take priority)
-        setup_kwargs['packages'] = find_namespace_packages(include=['airflow*'])
+    def include_provider_namespace_packages_when_installing_from_sources() -> None:

Review comment:
       The main reason was that I wanted to  name it appropriately. I think everywhere in a code (especially in such case where you do not care about a performance), if you have a need to write a paragraph of comment, it's much better to replace it withe a function that have the comment converted to pydoc, and is named appropriately, even if the code is short (but not obvious why we are doing it).
   
   This way when you look at the code you can focus on what the function does (fucntion name), why we do it (pydoc when for example hovering your mouse over the function) and how it does it (function body) separately and understand it at different levels. 




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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



##########
File path: docs/apache-airflow-providers/index.rst
##########
@@ -87,9 +88,24 @@ The capabilities are:
   connections defined by the provider. See :doc:`apache-airflow:howto/connection` for a description of
   connection and what capabilities of custom connection you can define.
 
+Custom provider packages
+''''''''''''''''''''''''
+
+However, there is more. You can develop your own providers. This is a bit involved, but your custom operators,
+hooks, sensors, transfer operators can be packaged together in a standard airflow package and installed
+using the same mechanisms. Moreover they can also use the same mechanisms to extend the Airflow Core with
+custom connections and extra operator links as described in the previous chapter. See
+[Developing custom providers](https://github.com/apache/airflow/blob/master/DEVELOPING_CUSTOM_PROVIDERS.rst)

Review comment:
       Right. It was to short for one file and I merged it in contributing, but did not update the reference :)




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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


   [The Workflow run](https://github.com/apache/airflow/actions/runs/469658599) is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on a change in pull request #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -780,76 +826,135 @@ def get_provider_package_from_package_id(package_id: str):
 
 
 class AirflowDistribution(Distribution):
-    """setuptools.Distribution subclass with Airflow specific behaviour"""
+    """
+    The setuptools.Distribution subclass with Airflow specific behaviour
+
+    The reason for pylint: disable=signature-differs of parse_config_files is explained here:
+    https://github.com/PyCQA/pylint/issues/3737
+
+    """
 
-    # https://github.com/PyCQA/pylint/issues/3737
     def parse_config_files(self, *args, **kwargs):  # pylint: disable=signature-differs
         """
         Ensure that when we have been asked to install providers from sources
-        that we don't *also* try to install those providers from PyPI
+        that we don't *also* try to install those providers from PyPI.
+        Also we should make sure that in this case we copy provider.yaml files so that
+        Providers manager can find package information.
         """
         super().parse_config_files(*args, **kwargs)
-        if os.getenv('INSTALL_PROVIDERS_FROM_SOURCES') == 'true':
+        if os.getenv(INSTALL_PROVIDERS_FROM_SOURCES) == 'true':
             self.install_requires = [  # noqa  pylint: disable=attribute-defined-outside-init
                 req for req in self.install_requires if not req.startswith('apache-airflow-providers-')
             ]
+            provider_yaml_files = glob.glob("airflow/providers/**/provider.yaml", recursive=True)
+            for provider_yaml_file in provider_yaml_files:
+                provider_relative_path = relpath(provider_yaml_file, os.path.join(my_dir, "airflow"))
+                self.package_data['airflow'].append(provider_relative_path)
         else:
             self.install_requires.extend(
                 [get_provider_package_from_package_id(package_id) for package_id in PREINSTALLED_PROVIDERS]
             )
 
 
-def add_provider_packages_to_requirements(extra_with_providers: str, providers: List[str]):
+def add_provider_packages_to_extras_requirements(extra: str, providers: List[str]) -> None:
     """
-    Adds provider packages to requirements
+    Adds provider packages to requirements of extra.
 
-    :param extra_with_providers: Name of the extra to add providers to
-    :param providers: list of provider names
+    :param extra: Name of the extra to add providers to
+    :param providers: list of provider ids
     """
-    EXTRAS_WITH_PROVIDERS.add(extra_with_providers)
-    EXTRAS_REQUIREMENTS[extra_with_providers].extend(
+    EXTRAS_WITH_PROVIDERS.add(extra)
+    EXTRAS_REQUIREMENTS[extra].extend(
         [get_provider_package_from_package_id(package_name) for package_name in providers]
     )
 
 
-def add_all_provider_packages():
+def add_all_provider_packages() -> None:
     """
-    In case of regular installation (when INSTALL_PROVIDERS_FROM_SOURCES is false), we should
-    add extra dependencies to Airflow - to get the providers automatically installed when
-    those extras are installed.
+    In case of regular installation (providers installed from packages), we should add extra dependencies to
+    Airflow - to get the providers automatically installed when those extras are installed.
+
+    For providers installed from sources we skip that step. That helps to test and install airflow with
+    all packages in CI - for example when new providers are added, otherwise the installation would fail
+    as the new provider is not yet in PyPI.
 
     """
     for provider in ALL_PROVIDERS:
-        add_provider_packages_to_requirements(provider, [provider])
-    add_provider_packages_to_requirements("all", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("devel_ci", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("devel_all", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("all_dbs", ALL_DB_PROVIDERS)
-    add_provider_packages_to_requirements("devel_hadoop", ["apache.hdfs", "apache.hive", "presto"])
+        add_provider_packages_to_extras_requirements(provider, [provider])
+    add_provider_packages_to_extras_requirements("all", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_ci", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_all", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("all_dbs", ALL_DB_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_hadoop", ["apache.hdfs", "apache.hive", "presto"])
+
+
+class Develop(develop_orig):
+    """Forces removal of providers in editable mode."""
+
+    def run(self):
+        self.announce('Installing in editable mode. Uninstalling provider packages!', level=log.INFO)
+        # We need to run "python3 -m pip" because it might be that older PIP binary is in the path
+        # And it results with an error when running pip directly (cannot import pip module)
+        # also PIP does not have a stable API so we have to run subprocesses ¯\_(ツ)_/¯
+        try:
+            installed_packages = (
+                subprocess.check_output(["python3", "-m", "pip", "freeze"]).decode().splitlines()
+            )
+            airflow_provider_packages = [
+                package_line.split("=")[0]
+                for package_line in installed_packages
+                if package_line.startswith("apache-airflow-providers")
+            ]
+            self.announce(f'Uninstalling ${airflow_provider_packages}!', level=log.INFO)
+            subprocess.check_call(["python3", "-m", "pip", "uninstall", "--yes", *airflow_provider_packages])
+        except subprocess.CalledProcessError as e:
+            self.announce(f'Error when uninstalling airflow provider packages: {e}!', level=log.WARN)
+        super().run()
+
+
+class Install(install_orig):
+    """Forces installation of providers from sources in editable mode."""
 
+    def run(self):
+        self.announce('Standard installation. Providers are installed from packages', level=log.INFO)

Review comment:
       It's only shown when `-vv` option is used with PIP I believe. This is a known problem that PIP is not really useful if you want to print any warning or information while installing because by default it only prints what authors of PIP wanted. I will double check it and remove (but I think I checked it before and it only showed up with high verbosity level).




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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


   


----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -597,10 +618,21 @@ def write_version(filename: str = os.path.join(*[my_dir, "airflow", "git_version
     'virtualenv': virtualenv,
 }
 
-# Add extras for all providers. For all providers the extras name = providers name
-for provider_name, provider_requirement in PROVIDERS_REQUIREMENTS.items():
-    EXTRAS_REQUIREMENTS[provider_name] = provider_requirement
+
+def add_extras_for_all_providers() -> None:
+    """
+    Adds extras for all providers.
+    By default all providers have the same extra name as provider id, for example
+    'apache.hive' extra has 'apache.hive' provider requirement.
+    :return:

Review comment:
       Yep




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] ashb commented on a change in pull request #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -780,76 +826,135 @@ def get_provider_package_from_package_id(package_id: str):
 
 
 class AirflowDistribution(Distribution):
-    """setuptools.Distribution subclass with Airflow specific behaviour"""
+    """
+    The setuptools.Distribution subclass with Airflow specific behaviour
+
+    The reason for pylint: disable=signature-differs of parse_config_files is explained here:
+    https://github.com/PyCQA/pylint/issues/3737
+
+    """
 
-    # https://github.com/PyCQA/pylint/issues/3737
     def parse_config_files(self, *args, **kwargs):  # pylint: disable=signature-differs
         """
         Ensure that when we have been asked to install providers from sources
-        that we don't *also* try to install those providers from PyPI
+        that we don't *also* try to install those providers from PyPI.
+        Also we should make sure that in this case we copy provider.yaml files so that
+        Providers manager can find package information.
         """
         super().parse_config_files(*args, **kwargs)
-        if os.getenv('INSTALL_PROVIDERS_FROM_SOURCES') == 'true':
+        if os.getenv(INSTALL_PROVIDERS_FROM_SOURCES) == 'true':
             self.install_requires = [  # noqa  pylint: disable=attribute-defined-outside-init
                 req for req in self.install_requires if not req.startswith('apache-airflow-providers-')
             ]
+            provider_yaml_files = glob.glob("airflow/providers/**/provider.yaml", recursive=True)
+            for provider_yaml_file in provider_yaml_files:
+                provider_relative_path = relpath(provider_yaml_file, os.path.join(my_dir, "airflow"))
+                self.package_data['airflow'].append(provider_relative_path)
         else:
             self.install_requires.extend(
                 [get_provider_package_from_package_id(package_id) for package_id in PREINSTALLED_PROVIDERS]
             )
 
 
-def add_provider_packages_to_requirements(extra_with_providers: str, providers: List[str]):
+def add_provider_packages_to_extras_requirements(extra: str, providers: List[str]) -> None:
     """
-    Adds provider packages to requirements
+    Adds provider packages to requirements of extra.
 
-    :param extra_with_providers: Name of the extra to add providers to
-    :param providers: list of provider names
+    :param extra: Name of the extra to add providers to
+    :param providers: list of provider ids
     """
-    EXTRAS_WITH_PROVIDERS.add(extra_with_providers)
-    EXTRAS_REQUIREMENTS[extra_with_providers].extend(
+    EXTRAS_WITH_PROVIDERS.add(extra)
+    EXTRAS_REQUIREMENTS[extra].extend(
         [get_provider_package_from_package_id(package_name) for package_name in providers]
     )
 
 
-def add_all_provider_packages():
+def add_all_provider_packages() -> None:
     """
-    In case of regular installation (when INSTALL_PROVIDERS_FROM_SOURCES is false), we should
-    add extra dependencies to Airflow - to get the providers automatically installed when
-    those extras are installed.
+    In case of regular installation (providers installed from packages), we should add extra dependencies to
+    Airflow - to get the providers automatically installed when those extras are installed.
+
+    For providers installed from sources we skip that step. That helps to test and install airflow with
+    all packages in CI - for example when new providers are added, otherwise the installation would fail
+    as the new provider is not yet in PyPI.
 
     """
     for provider in ALL_PROVIDERS:
-        add_provider_packages_to_requirements(provider, [provider])
-    add_provider_packages_to_requirements("all", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("devel_ci", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("devel_all", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("all_dbs", ALL_DB_PROVIDERS)
-    add_provider_packages_to_requirements("devel_hadoop", ["apache.hdfs", "apache.hive", "presto"])
+        add_provider_packages_to_extras_requirements(provider, [provider])
+    add_provider_packages_to_extras_requirements("all", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_ci", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_all", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("all_dbs", ALL_DB_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_hadoop", ["apache.hdfs", "apache.hive", "presto"])
+
+
+class Develop(develop_orig):
+    """Forces removal of providers in editable mode."""
+
+    def run(self):
+        self.announce('Installing in editable mode. Uninstalling provider packages!', level=log.INFO)
+        # We need to run "python3 -m pip" because it might be that older PIP binary is in the path
+        # And it results with an error when running pip directly (cannot import pip module)
+        # also PIP does not have a stable API so we have to run subprocesses ¯\_(ツ)_/¯
+        try:
+            installed_packages = (
+                subprocess.check_output(["python3", "-m", "pip", "freeze"]).decode().splitlines()
+            )
+            airflow_provider_packages = [
+                package_line.split("=")[0]
+                for package_line in installed_packages
+                if package_line.startswith("apache-airflow-providers")
+            ]
+            self.announce(f'Uninstalling ${airflow_provider_packages}!', level=log.INFO)
+            subprocess.check_call(["python3", "-m", "pip", "uninstall", "--yes", *airflow_provider_packages])
+        except subprocess.CalledProcessError as e:
+            self.announce(f'Error when uninstalling airflow provider packages: {e}!', level=log.WARN)
+        super().run()
+
+
+class Install(install_orig):
+    """Forces installation of providers from sources in editable mode."""
 
+    def run(self):
+        self.announce('Standard installation. Providers are installed from packages', level=log.INFO)
+        super().run()
 
-def do_setup():
-    """Perform the Airflow package setup."""
+
+def do_setup() -> None:
+    """
+    Perform the Airflow package setup.
+    Most values come from setup.cfg, only the dynamically calculated ones are passed to setup

Review comment:
       ```suggestion
       Perform the Airflow package setup.
       
       Most values come from setup.cfg, only the dynamically calculated ones are passed to setup
   ```




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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


   [The Workflow run](https://github.com/apache/airflow/actions/runs/458219496) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] kaxil commented on a change in pull request #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -150,13 +162,22 @@ def write_version(filename: str = os.path.join(*[my_dir, "airflow", "git_version
         file.write(text)
 
 
-if os.environ.get('USE_THEME_FROM_GIT'):
-    _SPHINX_AIRFLOW_THEME_URL = (
-        "@ https://github.com/apache/airflow-site/releases/download/0.0.4/"
-        "sphinx_airflow_theme-0.0.4-py3-none-any.whl"
-    )
-else:
-    _SPHINX_AIRFLOW_THEME_URL = ''
+def get_sphinx_theme_version() -> str:
+    """
+    Return sphinx theme version. If USE_THEME_FROM_GIT env variable is set, the theme is used from
+    github tp allow dynamically update it during development. However for regular PIP release

Review comment:
       ```suggestion
       GitHub to dynamically update it during development. However for regular PIP release
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] ashb commented on a change in pull request #13439: Forces unistalling providers in editable mode.

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



##########
File path: docs/apache-airflow-providers/index.rst
##########
@@ -119,6 +132,43 @@ When you write your own provider, consider following the
 `Naming conventions for provider packages <https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#naming-conventions-for-provider-packages>`_
 
 
+Community-managed vs. 3rd party providers
+'''''''''''''''''''''''''''''''''''''''''
+
+While you can develop your own providers, Apache Airflow has 60+ providers that are managed by the community.
+They are part of the same repository as Apache Airflow (we use ``monorepo`` approach where different
+parts of the system are developed in the same repository but then they are packaged and released separately).
+All the community-managed providers are in 'airflow/providers' folder and they are all sub-packages of
+'airflow.providers' package. All the providers are available as ``apache-airflow-providers-<PROVIDER_ID>``
+packages.
+
+The capabilities of the community-managed providers are the same as the third-party ones are the same. When
+the providers are installed from PyPI, they provide the entry-point containing the metadata as described
+in the previous chapter. However when they are locally developed, together with Airflow, the mechanism

Review comment:
       This document is user-facing, so talking about installing providers from sources feels out of place here.




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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


   This is much nicer way to treat both- editable mode of installation and the non-editable mode with `INSTALL_PROVIDERS_FROM_SOURCES=true`.  No more warnings for editable mode :)


----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -59,7 +68,10 @@ def finalize_options(self):
     def run(self):  # noqa
         """Run command to remove temporary files and directories."""
         os.chdir(my_dir)
-        os.system('rm -vrf ./build ./dist ./*.pyc ./*.tgz ./*.egg-info')
+        os.system(
+            'rm -vrf ./build ./dist ./*.pyc ./*.tgz ./*.egg-info '
+            + './docker-context-files/*.whl ./docker-context-files/*.tgz'  # noqa
+        )

Review comment:
       ```suggestion
           os.system(
               'rm -vrf ./build ./dist ./*.pyc ./*.tgz ./*.egg-info '
               './docker-context-files/*.whl ./docker-context-files/*.tgz'
           )
   ```
   
   This should work right? and we can remove `noqa` ?




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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



##########
File path: docs/apache-airflow-providers/index.rst
##########
@@ -119,6 +132,43 @@ When you write your own provider, consider following the
 `Naming conventions for provider packages <https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#naming-conventions-for-provider-packages>`_
 
 
+Community-managed vs. 3rd party providers
+'''''''''''''''''''''''''''''''''''''''''
+
+While you can develop your own providers, Apache Airflow has 60+ providers that are managed by the community.
+They are part of the same repository as Apache Airflow (we use ``monorepo`` approach where different
+parts of the system are developed in the same repository but then they are packaged and released separately).
+All the community-managed providers are in 'airflow/providers' folder and they are all sub-packages of
+'airflow.providers' package. All the providers are available as ``apache-airflow-providers-<PROVIDER_ID>``
+packages.
+
+The capabilities of the community-managed providers are the same as the third-party ones are the same. When
+the providers are installed from PyPI, they provide the entry-point containing the metadata as described
+in the previous chapter. However when they are locally developed, together with Airflow, the mechanism

Review comment:
       Good point. I will make a separate CUSTOM_PROVIDERS.rst document in the sources.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] turbaszek commented on a change in pull request #13439: Forces unistalling providers in editable mode.

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



##########
File path: INSTALL
##########
@@ -51,24 +51,35 @@ pip install . \
   --constraint "https://raw.githubusercontent.com/apache/airflow/constraints-master/constraints-3.6.txt"
 
 
-By default `pip install` in Airflow 2.0 installs only the provider packages that are needed by the extras,
-however if you want to install all providers (which was default behaviour in 1.10.*)
-you can do it by setting environment variable INSTALL_PROVIDERS_FROM_SOURCES to `true`.
+By default `pip install` in Airflow 2.0 installs only the provider packages that are needed by the extras and
+install them as packages from PyPI rather than from local sources:
 
 
-INSTALL_PROVIDERS_FROM_SOURCES="true" pip install . \
+pip install . \
   --constraint "https://raw.githubusercontent.com/apache/airflow/constraints-master/constraints-3.6.txt"
 
 
-You can also install airflow in "editable mode" (with -e) flag and then provider packages will be
-available, because they are used directly from the airflow sources:
-
+You can also install airflow in "editable mode" (with -e) flag and then provider packages are
+available directly from the sources (and the provider packages installed from PyPI are uninstalled in
+order to avoid having providers in two places. This is useful if you want to develop providers:
 
 pip install -e . \
   --constraint "https://raw.githubusercontent.com/apache/airflow/constraints-master/constraints-3.6.txt"
 
+You can als skip installing provider packages from PyPI by setting INSTALL_PROVIDERS_FROM_SOURCE to "true".
+In this case Airflow will be installed in non-editable mode with all providers installed from the sources.
+Additionally provider.yaml files will also be copied to providers folders which will make the providers

Review comment:
       The `provider.yaml` file is not described here so users will not know what it means, we should either said what it is, or remove the reference.
   
   Btw. isn't it `providers.yaml` (plural)?




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -150,13 +162,22 @@ def write_version(filename: str = os.path.join(*[my_dir, "airflow", "git_version
         file.write(text)
 
 
-if os.environ.get('USE_THEME_FROM_GIT'):
-    _SPHINX_AIRFLOW_THEME_URL = (
-        "@ https://github.com/apache/airflow-site/releases/download/0.0.4/"
-        "sphinx_airflow_theme-0.0.4-py3-none-any.whl"
-    )
-else:
-    _SPHINX_AIRFLOW_THEME_URL = ''
+def get_sphinx_theme_version() -> str:
+    """
+    Return sphinx theme version. If USE_THEME_FROM_GIT env variable is set, the theme is used from
+    github tp allow dynamically update it during development. However for regular PIP release

Review comment:
       argh




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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


   [The Workflow run](https://github.com/apache/airflow/actions/runs/469314455) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on a change in pull request #13439: Forces unistalling providers in editable mode.

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



##########
File path: INSTALL
##########
@@ -51,24 +51,41 @@ pip install . \
   --constraint "https://raw.githubusercontent.com/apache/airflow/constraints-master/constraints-3.6.txt"
 
 
-By default `pip install` in Airflow 2.0 installs only the provider packages that are needed by the extras,
-however if you want to install all providers (which was default behaviour in 1.10.*)
-you can do it by setting environment variable INSTALL_PROVIDERS_FROM_SOURCES to `true`.
+By default `pip install` in Airflow 2.0 installs only the provider packages that are needed by the extras and
+install them as packages from PyPI rather than from local sources:
 
 
-INSTALL_PROVIDERS_FROM_SOURCES="true" pip install . \
+pip install . \
   --constraint "https://raw.githubusercontent.com/apache/airflow/constraints-master/constraints-3.6.txt"
 
 
-You can also install airflow in "editable mode" (with -e) flag and then provider packages will be
-available, because they are used directly from the airflow sources:
+You can also install airflow in "editable mode" (with -e) flag and then provider packages are
+available directly from the sources (and the provider packages installed from PyPI are UNINSTALLED in
+order to avoid having providers in two places. And `provider.yaml` files are used to discover capabilities
+of the providers which are part of the airflow source code.
+
+You can read more about `provider.yaml` and community-managed providers in
+https://airflow.apache.org/docs/apache-airflow-providers/index.html
 
+This is useful if you want to develop providers:
 
 pip install -e . \
   --constraint "https://raw.githubusercontent.com/apache/airflow/constraints-master/constraints-3.6.txt"
 
+You can als skip installing provider packages from PyPI by setting INSTALL_PROVIDERS_FROM_SOURCE to "true".
+In this case Airflow will be installed in non-editable mode with all providers installed from the sources.
+Additionally `provider.yaml` files will also be copied to providers folders which will make the providers
+discoverable by Airflow even if they are not installed from packages in this case.
+
+INSTALL_PROVIDERS_FROM_SOURCES="true" pip install . \
+  --constraint "https://raw.githubusercontent.com/apache/airflow/constraints-master/constraints-3.6.txt"
+
+Airflow can be installed with extras to install some additional features (for example 'async' or 'doc' or
+to install automatically providers and all dependencies needed by that provider:
+
+pip install .[async,google,amazon] \
+  --constraint "https://raw.githubusercontent.com/apache/airflow/constraints-master/constraints-3.6.txt"
 
-# You can also install Airflow with extras specified. The list of available extras:

Review comment:
       Will do.




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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


   [The Workflow run](https://github.com/apache/airflow/actions/runs/457934876) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] kaxil commented on a change in pull request #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -780,76 +826,135 @@ def get_provider_package_from_package_id(package_id: str):
 
 
 class AirflowDistribution(Distribution):
-    """setuptools.Distribution subclass with Airflow specific behaviour"""
+    """
+    The setuptools.Distribution subclass with Airflow specific behaviour
+
+    The reason for pylint: disable=signature-differs of parse_config_files is explained here:
+    https://github.com/PyCQA/pylint/issues/3737
+
+    """
 
-    # https://github.com/PyCQA/pylint/issues/3737
     def parse_config_files(self, *args, **kwargs):  # pylint: disable=signature-differs
         """
         Ensure that when we have been asked to install providers from sources
-        that we don't *also* try to install those providers from PyPI
+        that we don't *also* try to install those providers from PyPI.
+        Also we should make sure that in this case we copy provider.yaml files so that
+        Providers manager can find package information.
         """
         super().parse_config_files(*args, **kwargs)
-        if os.getenv('INSTALL_PROVIDERS_FROM_SOURCES') == 'true':
+        if os.getenv(INSTALL_PROVIDERS_FROM_SOURCES) == 'true':
             self.install_requires = [  # noqa  pylint: disable=attribute-defined-outside-init
                 req for req in self.install_requires if not req.startswith('apache-airflow-providers-')
             ]
+            provider_yaml_files = glob.glob("airflow/providers/**/provider.yaml", recursive=True)
+            for provider_yaml_file in provider_yaml_files:
+                provider_relative_path = relpath(provider_yaml_file, os.path.join(my_dir, "airflow"))
+                self.package_data['airflow'].append(provider_relative_path)
         else:
             self.install_requires.extend(
                 [get_provider_package_from_package_id(package_id) for package_id in PREINSTALLED_PROVIDERS]
             )
 
 
-def add_provider_packages_to_requirements(extra_with_providers: str, providers: List[str]):
+def add_provider_packages_to_extras_requirements(extra: str, providers: List[str]) -> None:
     """
-    Adds provider packages to requirements
+    Adds provider packages to requirements of extra.
 
-    :param extra_with_providers: Name of the extra to add providers to
-    :param providers: list of provider names
+    :param extra: Name of the extra to add providers to
+    :param providers: list of provider ids
     """
-    EXTRAS_WITH_PROVIDERS.add(extra_with_providers)
-    EXTRAS_REQUIREMENTS[extra_with_providers].extend(
+    EXTRAS_WITH_PROVIDERS.add(extra)
+    EXTRAS_REQUIREMENTS[extra].extend(
         [get_provider_package_from_package_id(package_name) for package_name in providers]
     )
 
 
-def add_all_provider_packages():
+def add_all_provider_packages() -> None:
     """
-    In case of regular installation (when INSTALL_PROVIDERS_FROM_SOURCES is false), we should
-    add extra dependencies to Airflow - to get the providers automatically installed when
-    those extras are installed.
+    In case of regular installation (providers installed from packages), we should add extra dependencies to
+    Airflow - to get the providers automatically installed when those extras are installed.
+
+    For providers installed from sources we skip that step. That helps to test and install airflow with
+    all packages in CI - for example when new providers are added, otherwise the installation would fail
+    as the new provider is not yet in PyPI.
 
     """
     for provider in ALL_PROVIDERS:
-        add_provider_packages_to_requirements(provider, [provider])
-    add_provider_packages_to_requirements("all", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("devel_ci", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("devel_all", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("all_dbs", ALL_DB_PROVIDERS)
-    add_provider_packages_to_requirements("devel_hadoop", ["apache.hdfs", "apache.hive", "presto"])
+        add_provider_packages_to_extras_requirements(provider, [provider])
+    add_provider_packages_to_extras_requirements("all", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_ci", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_all", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("all_dbs", ALL_DB_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_hadoop", ["apache.hdfs", "apache.hive", "presto"])
+
+
+class Develop(develop_orig):
+    """Forces removal of providers in editable mode."""
+
+    def run(self):
+        self.announce('Installing in editable mode. Uninstalling provider packages!', level=log.INFO)
+        # We need to run "python3 -m pip" because it might be that older PIP binary is in the path
+        # And it results with an error when running pip directly (cannot import pip module)
+        # also PIP does not have a stable API so we have to run subprocesses ¯\_(ツ)_/¯
+        try:
+            installed_packages = (
+                subprocess.check_output(["python3", "-m", "pip", "freeze"]).decode().splitlines()
+            )
+            airflow_provider_packages = [
+                package_line.split("=")[0]
+                for package_line in installed_packages
+                if package_line.startswith("apache-airflow-providers")
+            ]
+            self.announce(f'Uninstalling ${airflow_provider_packages}!', level=log.INFO)
+            subprocess.check_call(["python3", "-m", "pip", "uninstall", "--yes", *airflow_provider_packages])
+        except subprocess.CalledProcessError as e:
+            self.announce(f'Error when uninstalling airflow provider packages: {e}!', level=log.WARN)
+        super().run()
+
+
+class Install(install_orig):
+    """Forces installation of providers from sources in editable mode."""
 
+    def run(self):
+        self.announce('Standard installation. Providers are installed from packages', level=log.INFO)
+        super().run()
 
-def do_setup():
-    """Perform the Airflow package setup."""
+
+def do_setup() -> None:
+    """
+    Perform the Airflow package setup.
+    Most values come from setup.cfg, only the dynamically calculated ones are passed to setup
+    function call. See https://setuptools.readthedocs.io/en/latest/userguide/declarative_config.html
+    """
     setup_kwargs = {}
 
-    if os.getenv('INSTALL_PROVIDERS_FROM_SOURCES') == 'true':
-        # Only specify this if we need this option, otherwise let default from
-        # setup.cfg control this (kwargs in setup() call take priority)
-        setup_kwargs['packages'] = find_namespace_packages(include=['airflow*'])
+    def include_provider_namespace_packages_when_installing_from_sources() -> None:
+        """
+        When install providers from sources we install all namespace packages found below airflow,

Review comment:
       ```suggestion
           When installing providers from sources we install all namespace packages found below airflow,
   ```




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -780,76 +826,135 @@ def get_provider_package_from_package_id(package_id: str):
 
 
 class AirflowDistribution(Distribution):
-    """setuptools.Distribution subclass with Airflow specific behaviour"""
+    """
+    The setuptools.Distribution subclass with Airflow specific behaviour
+
+    The reason for pylint: disable=signature-differs of parse_config_files is explained here:
+    https://github.com/PyCQA/pylint/issues/3737
+
+    """
 
-    # https://github.com/PyCQA/pylint/issues/3737
     def parse_config_files(self, *args, **kwargs):  # pylint: disable=signature-differs
         """
         Ensure that when we have been asked to install providers from sources
-        that we don't *also* try to install those providers from PyPI
+        that we don't *also* try to install those providers from PyPI.
+        Also we should make sure that in this case we copy provider.yaml files so that
+        Providers manager can find package information.
         """
         super().parse_config_files(*args, **kwargs)
-        if os.getenv('INSTALL_PROVIDERS_FROM_SOURCES') == 'true':
+        if os.getenv(INSTALL_PROVIDERS_FROM_SOURCES) == 'true':
             self.install_requires = [  # noqa  pylint: disable=attribute-defined-outside-init
                 req for req in self.install_requires if not req.startswith('apache-airflow-providers-')
             ]
+            provider_yaml_files = glob.glob("airflow/providers/**/provider.yaml", recursive=True)
+            for provider_yaml_file in provider_yaml_files:
+                provider_relative_path = relpath(provider_yaml_file, os.path.join(my_dir, "airflow"))
+                self.package_data['airflow'].append(provider_relative_path)
         else:
             self.install_requires.extend(
                 [get_provider_package_from_package_id(package_id) for package_id in PREINSTALLED_PROVIDERS]
             )
 
 
-def add_provider_packages_to_requirements(extra_with_providers: str, providers: List[str]):
+def add_provider_packages_to_extras_requirements(extra: str, providers: List[str]) -> None:
     """
-    Adds provider packages to requirements
+    Adds provider packages to requirements of extra.
 
-    :param extra_with_providers: Name of the extra to add providers to
-    :param providers: list of provider names
+    :param extra: Name of the extra to add providers to
+    :param providers: list of provider ids
     """
-    EXTRAS_WITH_PROVIDERS.add(extra_with_providers)
-    EXTRAS_REQUIREMENTS[extra_with_providers].extend(
+    EXTRAS_WITH_PROVIDERS.add(extra)
+    EXTRAS_REQUIREMENTS[extra].extend(
         [get_provider_package_from_package_id(package_name) for package_name in providers]
     )
 
 
-def add_all_provider_packages():
+def add_all_provider_packages() -> None:
     """
-    In case of regular installation (when INSTALL_PROVIDERS_FROM_SOURCES is false), we should
-    add extra dependencies to Airflow - to get the providers automatically installed when
-    those extras are installed.
+    In case of regular installation (providers installed from packages), we should add extra dependencies to
+    Airflow - to get the providers automatically installed when those extras are installed.
+
+    For providers installed from sources we skip that step. That helps to test and install airflow with
+    all packages in CI - for example when new providers are added, otherwise the installation would fail
+    as the new provider is not yet in PyPI.
 
     """
     for provider in ALL_PROVIDERS:
-        add_provider_packages_to_requirements(provider, [provider])
-    add_provider_packages_to_requirements("all", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("devel_ci", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("devel_all", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("all_dbs", ALL_DB_PROVIDERS)
-    add_provider_packages_to_requirements("devel_hadoop", ["apache.hdfs", "apache.hive", "presto"])
+        add_provider_packages_to_extras_requirements(provider, [provider])
+    add_provider_packages_to_extras_requirements("all", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_ci", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_all", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("all_dbs", ALL_DB_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_hadoop", ["apache.hdfs", "apache.hive", "presto"])
+
+
+class Develop(develop_orig):
+    """Forces removal of providers in editable mode."""
+
+    def run(self):
+        self.announce('Installing in editable mode. Uninstalling provider packages!', level=log.INFO)
+        # We need to run "python3 -m pip" because it might be that older PIP binary is in the path
+        # And it results with an error when running pip directly (cannot import pip module)
+        # also PIP does not have a stable API so we have to run subprocesses ¯\_(ツ)_/¯
+        try:
+            installed_packages = (
+                subprocess.check_output(["python3", "-m", "pip", "freeze"]).decode().splitlines()
+            )
+            airflow_provider_packages = [
+                package_line.split("=")[0]
+                for package_line in installed_packages
+                if package_line.startswith("apache-airflow-providers")
+            ]
+            self.announce(f'Uninstalling ${airflow_provider_packages}!', level=log.INFO)
+            subprocess.check_call(["python3", "-m", "pip", "uninstall", "--yes", *airflow_provider_packages])
+        except subprocess.CalledProcessError as e:
+            self.announce(f'Error when uninstalling airflow provider packages: {e}!', level=log.WARN)
+        super().run()
+
+
+class Install(install_orig):
+    """Forces installation of providers from sources in editable mode."""
 
+    def run(self):
+        self.announce('Standard installation. Providers are installed from packages', level=log.INFO)
+        super().run()
 
-def do_setup():
-    """Perform the Airflow package setup."""
+
+def do_setup() -> None:
+    """
+    Perform the Airflow package setup.
+    Most values come from setup.cfg, only the dynamically calculated ones are passed to setup
+    function call. See https://setuptools.readthedocs.io/en/latest/userguide/declarative_config.html
+    """
     setup_kwargs = {}
 
-    if os.getenv('INSTALL_PROVIDERS_FROM_SOURCES') == 'true':
-        # Only specify this if we need this option, otherwise let default from
-        # setup.cfg control this (kwargs in setup() call take priority)
-        setup_kwargs['packages'] = find_namespace_packages(include=['airflow*'])
+    def include_provider_namespace_packages_when_installing_from_sources() -> None:

Review comment:
       The main reason was that I wanted to  name it appropriately. I think everywhere in a code (especially in such case where you do not care about a performance, if you have a need to write a paragraph of comment, it's much better to replace it withe a function that have the comment converted to pydoc, and is named appropriately, even if the code is short (but not obvious why we are doing it).
   
   This way when you look at the code you can focus on what the function does (fucntion name), why we do it (pydoc when for example hovering your mouse over the function) and how it does it (function body) separately and understand it at different levels. 




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -780,76 +826,135 @@ def get_provider_package_from_package_id(package_id: str):
 
 
 class AirflowDistribution(Distribution):
-    """setuptools.Distribution subclass with Airflow specific behaviour"""
+    """
+    The setuptools.Distribution subclass with Airflow specific behaviour
+
+    The reason for pylint: disable=signature-differs of parse_config_files is explained here:
+    https://github.com/PyCQA/pylint/issues/3737
+
+    """
 
-    # https://github.com/PyCQA/pylint/issues/3737
     def parse_config_files(self, *args, **kwargs):  # pylint: disable=signature-differs
         """
         Ensure that when we have been asked to install providers from sources
-        that we don't *also* try to install those providers from PyPI
+        that we don't *also* try to install those providers from PyPI.
+        Also we should make sure that in this case we copy provider.yaml files so that
+        Providers manager can find package information.
         """
         super().parse_config_files(*args, **kwargs)
-        if os.getenv('INSTALL_PROVIDERS_FROM_SOURCES') == 'true':
+        if os.getenv(INSTALL_PROVIDERS_FROM_SOURCES) == 'true':
             self.install_requires = [  # noqa  pylint: disable=attribute-defined-outside-init
                 req for req in self.install_requires if not req.startswith('apache-airflow-providers-')
             ]
+            provider_yaml_files = glob.glob("airflow/providers/**/provider.yaml", recursive=True)
+            for provider_yaml_file in provider_yaml_files:
+                provider_relative_path = relpath(provider_yaml_file, os.path.join(my_dir, "airflow"))
+                self.package_data['airflow'].append(provider_relative_path)
         else:
             self.install_requires.extend(
                 [get_provider_package_from_package_id(package_id) for package_id in PREINSTALLED_PROVIDERS]
             )
 
 
-def add_provider_packages_to_requirements(extra_with_providers: str, providers: List[str]):
+def add_provider_packages_to_extras_requirements(extra: str, providers: List[str]) -> None:
     """
-    Adds provider packages to requirements
+    Adds provider packages to requirements of extra.
 
-    :param extra_with_providers: Name of the extra to add providers to
-    :param providers: list of provider names
+    :param extra: Name of the extra to add providers to
+    :param providers: list of provider ids
     """
-    EXTRAS_WITH_PROVIDERS.add(extra_with_providers)
-    EXTRAS_REQUIREMENTS[extra_with_providers].extend(
+    EXTRAS_WITH_PROVIDERS.add(extra)
+    EXTRAS_REQUIREMENTS[extra].extend(
         [get_provider_package_from_package_id(package_name) for package_name in providers]
     )
 
 
-def add_all_provider_packages():
+def add_all_provider_packages() -> None:
     """
-    In case of regular installation (when INSTALL_PROVIDERS_FROM_SOURCES is false), we should
-    add extra dependencies to Airflow - to get the providers automatically installed when
-    those extras are installed.
+    In case of regular installation (providers installed from packages), we should add extra dependencies to
+    Airflow - to get the providers automatically installed when those extras are installed.
+
+    For providers installed from sources we skip that step. That helps to test and install airflow with
+    all packages in CI - for example when new providers are added, otherwise the installation would fail
+    as the new provider is not yet in PyPI.
 
     """
     for provider in ALL_PROVIDERS:
-        add_provider_packages_to_requirements(provider, [provider])
-    add_provider_packages_to_requirements("all", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("devel_ci", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("devel_all", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("all_dbs", ALL_DB_PROVIDERS)
-    add_provider_packages_to_requirements("devel_hadoop", ["apache.hdfs", "apache.hive", "presto"])
+        add_provider_packages_to_extras_requirements(provider, [provider])
+    add_provider_packages_to_extras_requirements("all", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_ci", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_all", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("all_dbs", ALL_DB_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_hadoop", ["apache.hdfs", "apache.hive", "presto"])
+
+
+class Develop(develop_orig):
+    """Forces removal of providers in editable mode."""
+
+    def run(self):
+        self.announce('Installing in editable mode. Uninstalling provider packages!', level=log.INFO)
+        # We need to run "python3 -m pip" because it might be that older PIP binary is in the path
+        # And it results with an error when running pip directly (cannot import pip module)
+        # also PIP does not have a stable API so we have to run subprocesses ¯\_(ツ)_/¯
+        try:
+            installed_packages = (
+                subprocess.check_output(["python3", "-m", "pip", "freeze"]).decode().splitlines()
+            )
+            airflow_provider_packages = [
+                package_line.split("=")[0]
+                for package_line in installed_packages
+                if package_line.startswith("apache-airflow-providers")
+            ]
+            self.announce(f'Uninstalling ${airflow_provider_packages}!', level=log.INFO)
+            subprocess.check_call(["python3", "-m", "pip", "uninstall", "--yes", *airflow_provider_packages])
+        except subprocess.CalledProcessError as e:
+            self.announce(f'Error when uninstalling airflow provider packages: {e}!', level=log.WARN)
+        super().run()
+
+
+class Install(install_orig):
+    """Forces installation of providers from sources in editable mode."""
 
+    def run(self):
+        self.announce('Standard installation. Providers are installed from packages', level=log.INFO)
+        super().run()
 
-def do_setup():
-    """Perform the Airflow package setup."""
+
+def do_setup() -> None:
+    """
+    Perform the Airflow package setup.
+    Most values come from setup.cfg, only the dynamically calculated ones are passed to setup
+    function call. See https://setuptools.readthedocs.io/en/latest/userguide/declarative_config.html
+    """
     setup_kwargs = {}
 
-    if os.getenv('INSTALL_PROVIDERS_FROM_SOURCES') == 'true':
-        # Only specify this if we need this option, otherwise let default from
-        # setup.cfg control this (kwargs in setup() call take priority)
-        setup_kwargs['packages'] = find_namespace_packages(include=['airflow*'])
+    def include_provider_namespace_packages_when_installing_from_sources() -> None:
+        """
+        When install providers from sources we install all namespace packages found below airflow,
+        including airflow and provider packages, otherwise defaults from setup.cfg control this.
+        The kwargs in setup() call override those that are specified in setup.cfg.
+        """
+        if os.getenv(INSTALL_PROVIDERS_FROM_SOURCES) == 'true':
+            setup_kwargs['packages'] = find_namespace_packages(include=['airflow*'])
+
+    include_provider_namespace_packages_when_installing_from_sources()
+    if os.getenv(INSTALL_PROVIDERS_FROM_SOURCES) == 'true':
+        print("Installing providers from sources. Skip adding providers as dependencies")
     else:
         add_all_provider_packages()
+
     write_version()
     setup(
         distclass=AirflowDistribution,
-        # Most values come from setup.cfg -- see
-        # https://setuptools.readthedocs.io/en/latest/userguide/declarative_config.html

Review comment:
       Yep. I rephrased it and put a bit more comprehensive information (with dynamic vs. static content) few lines above or below.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] ashb commented on a change in pull request #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -640,15 +672,21 @@ def find_requirements_for_alias(alias_to_look_for: Tuple[str, str]) -> List[str]
         raise Exception(f"The extra {new_extra} is missing for alias {deprecated_extra}")
 
 
-# Add extras for all deprecated aliases. Requirements for those deprecated aliases are the same
-# as the extras they are replaced with
-for alias, extra in EXTRAS_DEPRECATED_ALIASES.items():
-    requirements = EXTRAS_REQUIREMENTS.get(extra) if extra != '' else []
-    if requirements is None:
-        raise Exception(f"The extra {extra} is missing for deprecated alias {alias}")
-    # Note the requirements are not copies - those are the same lists as for the new extras. This is intended.
-    # Thanks to that if the original extras are later extended with providers, aliases are extended as well.
-    EXTRAS_REQUIREMENTS[alias] = requirements
+def add_extras_for_all_deprecated_aliases() -> None:
+    """
+    Add extras for all deprecated aliases. Requirements for those deprecated aliases are the same
+    as the extras they are replaced with.
+    The requirements are not copies - those are the same lists as for the new extras. This is intended.
+    Thanks to that if the original extras are later extended with providers, aliases are extended as well.
+    """
+    for alias, extra in EXTRAS_DEPRECATED_ALIASES.items():
+        requirements = EXTRAS_REQUIREMENTS.get(extra) if extra != '' else []
+        if requirements is None:
+            raise Exception(f"The extra {extra} is missing for deprecated alias {alias}")
+        EXTRAS_REQUIREMENTS[alias] = requirements
+
+
+add_extras_for_all_deprecated_aliases()

Review comment:
       Just thinking there are some cases where we `import setup` and might not want these called.
   
   But if that isn't the case and everywhere we import it we'd want these called then we can leave this as it 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



[GitHub] [airflow] potiuk commented on a change in pull request #13439: Forces unistalling providers in editable mode.

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



##########
File path: docs/apache-airflow-providers/index.rst
##########
@@ -119,6 +132,43 @@ When you write your own provider, consider following the
 `Naming conventions for provider packages <https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#naming-conventions-for-provider-packages>`_
 
 
+Community-managed vs. 3rd party providers
+'''''''''''''''''''''''''''''''''''''''''
+
+While you can develop your own providers, Apache Airflow has 60+ providers that are managed by the community.
+They are part of the same repository as Apache Airflow (we use ``monorepo`` approach where different
+parts of the system are developed in the same repository but then they are packaged and released separately).
+All the community-managed providers are in 'airflow/providers' folder and they are all sub-packages of
+'airflow.providers' package. All the providers are available as ``apache-airflow-providers-<PROVIDER_ID>``
+packages.
+
+The capabilities of the community-managed providers are the same as the third-party ones are the same. When
+the providers are installed from PyPI, they provide the entry-point containing the metadata as described
+in the previous chapter. However when they are locally developed, together with Airflow, the mechanism

Review comment:
       Good point. I will make a separate CUSTOM_PROVIDERS.rst document.




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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


   


----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -150,13 +162,22 @@ def write_version(filename: str = os.path.join(*[my_dir, "airflow", "git_version
         file.write(text)
 
 
-if os.environ.get('USE_THEME_FROM_GIT'):
-    _SPHINX_AIRFLOW_THEME_URL = (
-        "@ https://github.com/apache/airflow-site/releases/download/0.0.4/"
-        "sphinx_airflow_theme-0.0.4-py3-none-any.whl"
-    )
-else:
-    _SPHINX_AIRFLOW_THEME_URL = ''
+def get_sphinx_theme_version() -> str:
+    """
+    Return sphinx theme version. If USE_THEME_FROM_GIT env variable is set, the theme is used from
+    github tp allow dynamically update it during development. However for regular PIP release
+    you cannot use @ package specification, so the latest available released theme package from
+    PIP is used.
+    :return: the
+    """
+    if os.environ.get('USE_THEME_FROM_GIT'):
+        return (
+            "@ https://github.com/apache/airflow-site/releases/download/0.0.4/"
+            + "sphinx_airflow_theme-0.0.4-py3-none-any.whl"

Review comment:
       ```suggestion
               "@ https://github.com/apache/airflow-site/releases/download/0.0.4/"
               "sphinx_airflow_theme-0.0.4-py3-none-any.whl"
   ```




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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


   [The Workflow run](https://github.com/apache/airflow/actions/runs/459102349) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] kaxil commented on a change in pull request #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -150,13 +162,22 @@ def write_version(filename: str = os.path.join(*[my_dir, "airflow", "git_version
         file.write(text)
 
 
-if os.environ.get('USE_THEME_FROM_GIT'):
-    _SPHINX_AIRFLOW_THEME_URL = (
-        "@ https://github.com/apache/airflow-site/releases/download/0.0.4/"
-        "sphinx_airflow_theme-0.0.4-py3-none-any.whl"
-    )
-else:
-    _SPHINX_AIRFLOW_THEME_URL = ''
+def get_sphinx_theme_version() -> str:
+    """
+    Return sphinx theme version. If USE_THEME_FROM_GIT env variable is set, the theme is used from
+    github tp allow dynamically update it during development. However for regular PIP release
+    you cannot use @ package specification, so the latest available released theme package from
+    PIP is used.
+    :return: the

Review comment:
       incomplete sentence!




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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


   cc: @dstandish . This should solve https://github.com/apache/airflow/discussions/13383#discussioncomment-255013


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] ashb commented on a change in pull request #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -640,15 +672,21 @@ def find_requirements_for_alias(alias_to_look_for: Tuple[str, str]) -> List[str]
         raise Exception(f"The extra {new_extra} is missing for alias {deprecated_extra}")
 
 
-# Add extras for all deprecated aliases. Requirements for those deprecated aliases are the same
-# as the extras they are replaced with
-for alias, extra in EXTRAS_DEPRECATED_ALIASES.items():
-    requirements = EXTRAS_REQUIREMENTS.get(extra) if extra != '' else []
-    if requirements is None:
-        raise Exception(f"The extra {extra} is missing for deprecated alias {alias}")
-    # Note the requirements are not copies - those are the same lists as for the new extras. This is intended.
-    # Thanks to that if the original extras are later extended with providers, aliases are extended as well.
-    EXTRAS_REQUIREMENTS[alias] = requirements
+def add_extras_for_all_deprecated_aliases() -> None:
+    """
+    Add extras for all deprecated aliases. Requirements for those deprecated aliases are the same
+    as the extras they are replaced with.
+    The requirements are not copies - those are the same lists as for the new extras. This is intended.
+    Thanks to that if the original extras are later extended with providers, aliases are extended as well.
+    """
+    for alias, extra in EXTRAS_DEPRECATED_ALIASES.items():
+        requirements = EXTRAS_REQUIREMENTS.get(extra) if extra != '' else []
+        if requirements is None:
+            raise Exception(f"The extra {extra} is missing for deprecated alias {alias}")
+        EXTRAS_REQUIREMENTS[alias] = requirements
+
+
+add_extras_for_all_deprecated_aliases()

Review comment:
       Do these need to be called at the top level, or could/should the be called in the "if __main__" block instead?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] ashb commented on a change in pull request #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -780,76 +826,135 @@ def get_provider_package_from_package_id(package_id: str):
 
 
 class AirflowDistribution(Distribution):
-    """setuptools.Distribution subclass with Airflow specific behaviour"""
+    """
+    The setuptools.Distribution subclass with Airflow specific behaviour
+
+    The reason for pylint: disable=signature-differs of parse_config_files is explained here:
+    https://github.com/PyCQA/pylint/issues/3737
+
+    """
 
-    # https://github.com/PyCQA/pylint/issues/3737
     def parse_config_files(self, *args, **kwargs):  # pylint: disable=signature-differs
         """
         Ensure that when we have been asked to install providers from sources
-        that we don't *also* try to install those providers from PyPI
+        that we don't *also* try to install those providers from PyPI.
+        Also we should make sure that in this case we copy provider.yaml files so that
+        Providers manager can find package information.
         """
         super().parse_config_files(*args, **kwargs)
-        if os.getenv('INSTALL_PROVIDERS_FROM_SOURCES') == 'true':
+        if os.getenv(INSTALL_PROVIDERS_FROM_SOURCES) == 'true':
             self.install_requires = [  # noqa  pylint: disable=attribute-defined-outside-init
                 req for req in self.install_requires if not req.startswith('apache-airflow-providers-')
             ]
+            provider_yaml_files = glob.glob("airflow/providers/**/provider.yaml", recursive=True)
+            for provider_yaml_file in provider_yaml_files:
+                provider_relative_path = relpath(provider_yaml_file, os.path.join(my_dir, "airflow"))
+                self.package_data['airflow'].append(provider_relative_path)
         else:
             self.install_requires.extend(
                 [get_provider_package_from_package_id(package_id) for package_id in PREINSTALLED_PROVIDERS]
             )
 
 
-def add_provider_packages_to_requirements(extra_with_providers: str, providers: List[str]):
+def add_provider_packages_to_extras_requirements(extra: str, providers: List[str]) -> None:
     """
-    Adds provider packages to requirements
+    Adds provider packages to requirements of extra.
 
-    :param extra_with_providers: Name of the extra to add providers to
-    :param providers: list of provider names
+    :param extra: Name of the extra to add providers to
+    :param providers: list of provider ids
     """
-    EXTRAS_WITH_PROVIDERS.add(extra_with_providers)
-    EXTRAS_REQUIREMENTS[extra_with_providers].extend(
+    EXTRAS_WITH_PROVIDERS.add(extra)
+    EXTRAS_REQUIREMENTS[extra].extend(
         [get_provider_package_from_package_id(package_name) for package_name in providers]
     )
 
 
-def add_all_provider_packages():
+def add_all_provider_packages() -> None:
     """
-    In case of regular installation (when INSTALL_PROVIDERS_FROM_SOURCES is false), we should
-    add extra dependencies to Airflow - to get the providers automatically installed when
-    those extras are installed.
+    In case of regular installation (providers installed from packages), we should add extra dependencies to
+    Airflow - to get the providers automatically installed when those extras are installed.
+
+    For providers installed from sources we skip that step. That helps to test and install airflow with
+    all packages in CI - for example when new providers are added, otherwise the installation would fail
+    as the new provider is not yet in PyPI.
 
     """
     for provider in ALL_PROVIDERS:
-        add_provider_packages_to_requirements(provider, [provider])
-    add_provider_packages_to_requirements("all", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("devel_ci", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("devel_all", ALL_PROVIDERS)
-    add_provider_packages_to_requirements("all_dbs", ALL_DB_PROVIDERS)
-    add_provider_packages_to_requirements("devel_hadoop", ["apache.hdfs", "apache.hive", "presto"])
+        add_provider_packages_to_extras_requirements(provider, [provider])
+    add_provider_packages_to_extras_requirements("all", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_ci", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_all", ALL_PROVIDERS)
+    add_provider_packages_to_extras_requirements("all_dbs", ALL_DB_PROVIDERS)
+    add_provider_packages_to_extras_requirements("devel_hadoop", ["apache.hdfs", "apache.hive", "presto"])
+
+
+class Develop(develop_orig):
+    """Forces removal of providers in editable mode."""
+
+    def run(self):
+        self.announce('Installing in editable mode. Uninstalling provider packages!', level=log.INFO)
+        # We need to run "python3 -m pip" because it might be that older PIP binary is in the path
+        # And it results with an error when running pip directly (cannot import pip module)
+        # also PIP does not have a stable API so we have to run subprocesses ¯\_(ツ)_/¯
+        try:
+            installed_packages = (
+                subprocess.check_output(["python3", "-m", "pip", "freeze"]).decode().splitlines()
+            )
+            airflow_provider_packages = [
+                package_line.split("=")[0]
+                for package_line in installed_packages
+                if package_line.startswith("apache-airflow-providers")
+            ]
+            self.announce(f'Uninstalling ${airflow_provider_packages}!', level=log.INFO)
+            subprocess.check_call(["python3", "-m", "pip", "uninstall", "--yes", *airflow_provider_packages])
+        except subprocess.CalledProcessError as e:
+            self.announce(f'Error when uninstalling airflow provider packages: {e}!', level=log.WARN)
+        super().run()
+
+
+class Install(install_orig):
+    """Forces installation of providers from sources in editable mode."""
 
+    def run(self):
+        self.announce('Standard installation. Providers are installed from packages', level=log.INFO)

Review comment:
       Ah if it only shows up under v>=2 that's alright then.




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -59,7 +68,10 @@ def finalize_options(self):
     def run(self):  # noqa
         """Run command to remove temporary files and directories."""
         os.chdir(my_dir)
-        os.system('rm -vrf ./build ./dist ./*.pyc ./*.tgz ./*.egg-info')
+        os.system(
+            'rm -vrf ./build ./dist ./*.pyc ./*.tgz ./*.egg-info '
+            + './docker-context-files/*.whl ./docker-context-files/*.tgz'  # noqa
+        )

Review comment:
       Nope. # noqa was there for another reason (security of os.system for user input) which was a false-positive, But I can double-check,




----------------------------------------------------------------
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 #13439: Forces unistalling providers in editable mode.

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


   [The Workflow run](https://github.com/apache/airflow/actions/runs/458202266) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] ashb commented on a change in pull request #13439: Forces unistalling providers in editable mode.

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



##########
File path: setup.py
##########
@@ -16,16 +16,25 @@
 # specific language governing permissions and limitations
 # under the License.
 """Setup.py for the Airflow project."""
-
+import glob
 import logging
 import os
 import subprocess
 import unittest
-from os.path import dirname
+from distutils import log
+from os.path import dirname, relpath
 from textwrap import wrap
 from typing import Dict, List, Set, Tuple
 
 from setuptools import Command, Distribution, find_namespace_packages, setup
+from setuptools.command.develop import develop as develop_orig
+from setuptools.command.install import install as install_orig
+
+# Controls whether providers are installed from pckages or directly from sources
+# It is turned on by default in case of development environments such as Breeze
+# And it is particularly useful when you add a new provider and there is no
+# PyPI version to install the provider package from
+INSTALL_PROVIDERS_FROM_SOURCES = 'INSTALL_PROVIDERS_FROM_SOURCES'

Review comment:
       Man I hate pylint sometimes.




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