You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/12/08 00:53:57 UTC

[GitHub] [airflow] potiuk opened a new pull request #12903: Adds predefined providers to install_requires.

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


   The 4 providers (http, ftp, sqlite, imap) are popular
   and they do not require any additionl dependencies so we decided
   to include them by default in Airflow 2.0
   
   This also means we have to come back to keeping install_requires
   in the setup.py because in development environment, we cannot add
   those providers as we want to keep them installed from sources
   rather than from packages.
   
   <!--
   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] ashb commented on pull request #12903: Adds predefined providers to install_requires.

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


   > Added the 'pre-installed providers' we agreed to . Unfortunately @ashb it looks like we have to go back with install_requires to setup.py because depending on dev/prod environment (INSTALL_PROVIDERS_FROM_SOURCES) we have to dynmically add them to install_requires (unless of course we figure our something else). Seems like install_requires in setup.py overrides the one that is in setup.cfg unfortunately.
   
   Let me have a look - see if there's anything we can 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 pull request #12903: Adds predefined providers to install_requires.

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


   Added the 'pre-installed providers' we agreed to . Unfortunately @ashb it looks like we have to go back with install_requires to setup.py because depending on dev/prod environment (INSTALL_PROVIDERS_FROM_SOURCES) we have to dynmically add them to install_requires (unless of course we figure our something else). Seems like install_requires in setup.py overrides the one that is in setup.cfg unfortunately.


----------------------------------------------------------------
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 #12903: Update comments in setup.py

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


   The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.


----------------------------------------------------------------
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 #12903: Update deprecation comments in setup.py

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



##########
File path: .pre-commit-config.yaml
##########
@@ -241,7 +241,7 @@ repos:
       - id: setup-order
         name: Check order of dependencies in setup.py
         language: python
-        files: ^setup.py$
+        files: ^setup.py$|^setup.cfg$

Review comment:
       We do. There is a check that checks sequence in setup.cfg as well :)




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

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



[GitHub] [airflow] potiuk commented on a change in pull request #12903: Update deprecation comments in setup.py

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



##########
File path: .pre-commit-config.yaml
##########
@@ -241,7 +241,7 @@ repos:
       - id: setup-order
         name: Check order of dependencies in setup.py
         language: python
-        files: ^setup.py$
+        files: ^setup.py$|^setup.cfg$

Review comment:
       You've updated it when you moved the "install_requires".




----------------------------------------------------------------
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 #12903: Update comments in setup.py

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


   Yep, #12916 does it much better ! I turned this one into comment-only change @ashb 


----------------------------------------------------------------
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 pull request #12903: Adds predefined providers to install_requires.

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


   Is the only case we have to deal with when you do `pip install -e .` right?


----------------------------------------------------------------
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 #12903: Adds predefined providers to install_requires.

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


   [The Workflow run](https://github.com/apache/airflow/actions/runs/407636401) 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 merged pull request #12903: Update deprecation comments in setup.py

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


   


----------------------------------------------------------------
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 pull request #12903: Adds predefined providers to install_requires.

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


   See https://github.com/apache/airflow/pull/12916 for an approach that things in setup.cfg


----------------------------------------------------------------
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 #12903: Update deprecation comments in setup.py

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



##########
File path: .pre-commit-config.yaml
##########
@@ -241,7 +241,7 @@ repos:
       - id: setup-order
         name: Check order of dependencies in setup.py
         language: python
-        files: ^setup.py$
+        files: ^setup.py$|^setup.cfg$

Review comment:
       But the comment + description of this commit + the files spec were not updated then. I modified the description now to be clear that 'setup-order' concerns both .py and .cfg




----------------------------------------------------------------
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 #12903: Update comments in setup.py

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



##########
File path: .pre-commit-config.yaml
##########
@@ -241,7 +241,7 @@ repos:
       - id: setup-order
         name: Check order of dependencies in setup.py
         language: python
-        files: ^setup.py$
+        files: ^setup.py$|^setup.cfg$

Review comment:
       Don't need this anymore do we?




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