You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "jedcunningham (via GitHub)" <gi...@apache.org> on 2024/05/08 18:45:27 UTC

[PR] Simplify `airflow_version` imports [airflow]

jedcunningham opened a new pull request, #39497:
URL: https://github.com/apache/airflow/pull/39497

   We don't need a fallback as this import works on 2.7+ (in fact, it seems to have worked even for 2.0 🤷).


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

Re: [PR] Simplify `airflow_version` imports [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #39497:
URL: https://github.com/apache/airflow/pull/39497#discussion_r1594517558


##########
airflow/providers/alibaba/__init__.py:
##########
@@ -29,10 +29,7 @@
 
 __version__ = "2.8.0"
 
-try:
-    from airflow import __version__ as airflow_version
-except ImportError:
-    from airflow.version import version as airflow_version
+from airflow import __version__ as airflow_version

Review Comment:
   Another comment.
   
   I think we should switch to "standard" way of getting airflow version. The `__version__` is non-standard (PEP about it has actually been rejected https://peps.python.org/pep-0396/) 
   
   I think we've been discussing it with @Taragolis before and https://docs.python.org/3/library/importlib.metadata.html is a better/more standard way:
   
   ```python
   from importlib.metadata import version  
   airflow_version = version('apache-airlfow')  
   ```
   
   https://docs.python.org/3/library/importlib.metadata.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.

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

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

Re: [PR] Simplify `airflow_version` imports [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #39497:
URL: https://github.com/apache/airflow/pull/39497#discussion_r1594517558


##########
airflow/providers/alibaba/__init__.py:
##########
@@ -29,10 +29,7 @@
 
 __version__ = "2.8.0"
 
-try:
-    from airflow import __version__ as airflow_version
-except ImportError:
-    from airflow.version import version as airflow_version
+from airflow import __version__ as airflow_version

Review Comment:
   Another comment.
   
   I think we should switch to "standard" way of getting airflow version. The `__version__` is non-standard (PEP about it has actually been rejected https://peps.python.org/pep-0396/) 
   
   I think we've been discussing it with @Taragolis before and https://docs.python.org/3/library/importlib.metadata.html is a better/more standard way:
   
   ```python
   from importlib.metadata import version  
   airflow_version = version('apache-airflow')  
   ```
   
   https://docs.python.org/3/library/importlib.metadata.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.

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

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

Re: [PR] Simplify `airflow_version` imports [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #39497:
URL: https://github.com/apache/airflow/pull/39497#issuecomment-2101248064

   BTW. I think we can simplify it even further. I think this one should work: 
   
   ```
   packaging.version.parse(airflow_version).base_version < packaging.version.parse("2.7.0")
   ```
   
   


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

Re: [PR] Simplify `airflow_version` imports [airflow]

Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on code in PR #39497:
URL: https://github.com/apache/airflow/pull/39497#discussion_r1594509148


##########
airflow/providers/airbyte/__init__.py:
##########
@@ -29,10 +29,7 @@
 
 __version__ = "3.8.0"
 
-try:

Review Comment:
   > BTW. It could be done by changing template and running breeze release-management prepare-provider-documentation --reapply-templates-only
   
   Oh, that would have been faster. Next time I'll keep that in mind.



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

Re: [PR] Simplify `airflow_version` imports [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #39497:
URL: https://github.com/apache/airflow/pull/39497#discussion_r1594519939


##########
airflow/providers/alibaba/__init__.py:
##########
@@ -29,10 +29,7 @@
 
 __version__ = "2.8.0"
 
-try:
-    from airflow import __version__ as airflow_version
-except ImportError:
-    from airflow.version import version as airflow_version
+from airflow import __version__ as airflow_version

Review Comment:
   As of Python 3.8 of course - but we are already 3.8, so ....



##########
airflow/providers/alibaba/__init__.py:
##########
@@ -29,10 +29,7 @@
 
 __version__ = "2.8.0"
 
-try:
-    from airflow import __version__ as airflow_version
-except ImportError:
-    from airflow.version import version as airflow_version
+from airflow import __version__ as airflow_version

Review Comment:
   As of Python 3.8 of course - but we are already 3.8+, so ....



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

Re: [PR] Simplify `airflow_version` imports [airflow]

Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on PR #39497:
URL: https://github.com/apache/airflow/pull/39497#issuecomment-2101299301

   > BTW. I think we can simplify it even further. I think this one should work:
   > 
   > ```
   > packaging.version.parse(airflow_version).base_version < packaging.version.parse("2.7.0")
   > ```
   
   Nope, no luck. `base_version` returns a str, so we need to parse it again to compare.


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

Re: [PR] Simplify `airflow_version` imports [airflow]

Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham merged PR #39497:
URL: https://github.com/apache/airflow/pull/39497


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

Re: [PR] Simplify `airflow_version` imports [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #39497:
URL: https://github.com/apache/airflow/pull/39497#discussion_r1594507299


##########
airflow/providers/airbyte/__init__.py:
##########
@@ -29,10 +29,7 @@
 
 __version__ = "3.8.0"
 
-try:

Review Comment:
   See comment few lines above:
   
   ```
   # IF YOU WANT TO MODIFY THIS FILE, YOU SHOULD MODIFY THE TEMPLATE
   # `PROVIDER__INIT__PY_TEMPLATE.py.jinja2` IN the `dev/breeze/src/airflow_breeze/templates` DIRECTORY
   ```
   
   Yoy need to update the template - otherwise generating provider documentation will automatically regenerete those __init__.py files.
   
   BTW. It could be done by changing template and running `breeze release-management prepare-provider-documentation --reapply-templates-only`
   
   Here: https://github.com/apache/airflow/blob/main/dev/breeze/src/airflow_breeze/templates/PROVIDER__INIT__PY_TEMPLATE.py.jinja2
   
   



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

Re: [PR] Simplify `airflow_version` imports [airflow]

Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on code in PR #39497:
URL: https://github.com/apache/airflow/pull/39497#discussion_r1594570487


##########
airflow/providers/alibaba/__init__.py:
##########
@@ -29,10 +29,7 @@
 
 __version__ = "2.8.0"
 
-try:
-    from airflow import __version__ as airflow_version
-except ImportError:
-    from airflow.version import version as airflow_version
+from airflow import __version__ as airflow_version

Review Comment:
   Good call, thanks. Note I didn't import `version` as I thought having a str `__version__` and a method `version` that comes from importlib felt a bit weird.



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

Re: [PR] Simplify `airflow_version` imports [airflow]

Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on code in PR #39497:
URL: https://github.com/apache/airflow/pull/39497#discussion_r1594508448


##########
airflow/providers/airbyte/__init__.py:
##########
@@ -29,10 +29,7 @@
 
 __version__ = "3.8.0"
 
-try:

Review Comment:
   I did, it's the very last file :)



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

Re: [PR] Simplify `airflow_version` imports [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #39497:
URL: https://github.com/apache/airflow/pull/39497#discussion_r1594508185


##########
airflow/providers/airbyte/__init__.py:
##########
@@ -29,10 +29,7 @@
 
 __version__ = "3.8.0"
 
-try:

Review Comment:
   Ups. I see it's been done :) 



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