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 2022/01/14 15:27:17 UTC

[GitHub] [airflow] kaxil opened a new pull request #20878: Unpin ``argcomplete`` and ``colorlog``

kaxil opened a new pull request #20878:
URL: https://github.com/apache/airflow/pull/20878


   For Both of these libraries, the only breaking changes are dropping support for old Python version.
   
   - https://github.com/kislyuk/argcomplete/blob/develop/Changes.rst
   - https://github.com/borntyping/python-colorlog/releases
   
   <!--
   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/main/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/main/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.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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



[GitHub] [airflow] ashb commented on a change in pull request #20878: Unpin ``argcomplete`` and ``colorlog``

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



##########
File path: setup.cfg
##########
@@ -81,14 +81,14 @@ setup_requires =
 #####################################################################################################
 install_requires =
     alembic>=1.5.1, <2.0
-    argcomplete~=1.10
+    argcomplete>=1.10

Review comment:
       `<3` too?




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

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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



[GitHub] [airflow] github-actions[bot] commented on pull request #20878: Unpin ``argcomplete`` and ``colorlog``

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


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

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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



[GitHub] [airflow] potiuk edited a comment on pull request #20878: Unpin ``argcomplete`` and ``colorlog``

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #20878:
URL: https://github.com/apache/airflow/pull/20878#issuecomment-1013732136


   I personally believe we should go this way (I am happy to codify this and discuss/vote it:
   
   1. No upper bound by default
   2. When we have good reasons to believe a future version will break things we can introduce upper bounds
   3. We should always have a comment explaining why we have upper bound and condition that should be fulfilled 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.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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



[GitHub] [airflow] ashb commented on a change in pull request #20878: Unpin ``argcomplete`` and ``colorlog``

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



##########
File path: setup.cfg
##########
@@ -81,14 +81,14 @@ setup_requires =
 #####################################################################################################
 install_requires =
     alembic>=1.5.1, <2.0
-    argcomplete~=1.10
+    argcomplete>=1.10
     attrs>=20.0, <21.0
     blinker
     cached_property~=1.5;python_version<="3.7"
     cattrs~=1.1, !=1.7.*
     # Required by vendored-in connexion
     clickclick>=1.2
-    colorlog>=4.0.2, <6.0
+    colorlog>=4.0.2

Review comment:
       And `<7`? WDYT?




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

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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



[GitHub] [airflow] potiuk commented on a change in pull request #20878: Unpin ``argcomplete`` and ``colorlog``

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



##########
File path: setup.cfg
##########
@@ -81,14 +81,14 @@ setup_requires =
 #####################################################################################################
 install_requires =
     alembic>=1.5.1, <2.0
-    argcomplete~=1.10
+    argcomplete>=1.10
     attrs>=20.0, <21.0
     blinker
     cached_property~=1.5;python_version<="3.7"
     cattrs~=1.1, !=1.7.*
     # Required by vendored-in connexion
     clickclick>=1.2
-    colorlog>=4.0.2, <6.0
+    colorlog>=4.0.2

Review comment:
       @ashb We actually only support installing airlfow with constraints. Any other use is not supported and we very clearly write about it in our https://airflow.apache.org/docs/apache-airflow/stable/installation/installing-from-pypi.html
   
   > While there are some successes with using other tools like poetry or pip-tools, they do not share the same workflow as pip - especially when it comes to constraint vs. requirements management. Installing via Poetry or pip-tools is not currently supported. If you wish to install airflow using those tools you should use the constraints and convert them to appropriate format and workflow that your tool requires.
   
   The way how we have "open" upper bound requirements for most of our dependencies pretty much mandates using constraints. And as far as I know there is no other solution if we do not want our users to be limited. 
   
   Unless we are able to define which constraints (and why) shoudl be limited, and which are not, I think deciding on some of those to be upper-limited is really arbitrary.
   
   I think we need to agree on some rules there rather than have different approach for different PRs/Constraints.
   
   I will ask a discussion on the devlist. 




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

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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



[GitHub] [airflow] kaxil commented on a change in pull request #20878: Unpin ``argcomplete`` and ``colorlog``

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



##########
File path: setup.cfg
##########
@@ -81,14 +81,14 @@ setup_requires =
 #####################################################################################################
 install_requires =
     alembic>=1.5.1, <2.0
-    argcomplete~=1.10
+    argcomplete>=1.10
     attrs>=20.0, <21.0
     blinker
     cached_property~=1.5;python_version<="3.7"
     cattrs~=1.1, !=1.7.*
     # Required by vendored-in connexion
     clickclick>=1.2
-    colorlog>=4.0.2, <6.0
+    colorlog>=4.0.2

Review comment:
       No strong opinion, I went back and forth with it before creating the PR but remove those as we recommend using constraints for installation anyway so keep the deps "open". 
   
   Pushed https://github.com/apache/airflow/pull/20878/commits/a83dba737b8685e4e9870af3b0ee1fc5cad82eb3 to add those upper bounds




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

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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



[GitHub] [airflow] kaxil merged pull request #20878: Unpin ``argcomplete`` and ``colorlog``

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


   


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

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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



[GitHub] [airflow] potiuk commented on pull request #20878: Unpin ``argcomplete`` and ``colorlog``

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


   I personally believe we should go this way (I am happy to codify this and discuss/vote it:
   
   1. No upper bound by default
   2. When we have good reasons to believe a future version will break things we can introduce it
   3. We should always have a comment explaining why we have upper bound and condition that should be fulfilled 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.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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



[GitHub] [airflow] kaxil commented on a change in pull request #20878: Unpin ``argcomplete`` and ``colorlog``

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



##########
File path: setup.cfg
##########
@@ -81,14 +81,14 @@ setup_requires =
 #####################################################################################################
 install_requires =
     alembic>=1.5.1, <2.0
-    argcomplete~=1.10
+    argcomplete>=1.10
     attrs>=20.0, <21.0
     blinker
     cached_property~=1.5;python_version<="3.7"
     cattrs~=1.1, !=1.7.*
     # Required by vendored-in connexion
     clickclick>=1.2
-    colorlog>=4.0.2, <6.0
+    colorlog>=4.0.2

Review comment:
       No strong opinion, I went back and forth with it before creating the PR but removed those as we recommend using constraints for installation anyway so keep the deps "open". 
   
   Pushed https://github.com/apache/airflow/pull/20878/commits/4bdd6cf1718db88ff02030b8a490b4436aee85f7 to add those upper bounds




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

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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



[GitHub] [airflow] kaxil commented on a change in pull request #20878: Unpin ``argcomplete`` and ``colorlog``

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



##########
File path: setup.cfg
##########
@@ -81,14 +81,14 @@ setup_requires =
 #####################################################################################################
 install_requires =
     alembic>=1.5.1, <2.0
-    argcomplete~=1.10
+    argcomplete>=1.10
     attrs>=20.0, <21.0
     blinker
     cached_property~=1.5;python_version<="3.7"
     cattrs~=1.1, !=1.7.*
     # Required by vendored-in connexion
     clickclick>=1.2
-    colorlog>=4.0.2, <6.0
+    colorlog>=4.0.2

Review comment:
       No strong opinion, I went back and forth with it before creating the PR but remove those as we recommend using constraints for installation anyway so keep the deps "open". 
   
   Pushed https://github.com/apache/airflow/pull/20878/commits/4bdd6cf1718db88ff02030b8a490b4436aee85f7 to add those upper bounds




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

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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



[GitHub] [airflow] ashb commented on a change in pull request #20878: Unpin ``argcomplete`` and ``colorlog``

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



##########
File path: setup.cfg
##########
@@ -81,14 +81,14 @@ setup_requires =
 #####################################################################################################
 install_requires =
     alembic>=1.5.1, <2.0
-    argcomplete~=1.10
+    argcomplete>=1.10
     attrs>=20.0, <21.0
     blinker
     cached_property~=1.5;python_version<="3.7"
     cattrs~=1.1, !=1.7.*
     # Required by vendored-in connexion
     clickclick>=1.2
-    colorlog>=4.0.2, <6.0
+    colorlog>=4.0.2

Review comment:
       "we recommend using constraints" -- recommend but don't require.




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

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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