Dev-iL commented on code in PR #37585:
URL: https://github.com/apache/airflow/pull/37585#discussion_r1497099587
##########
airflow/utils/pydantic.py:
##########
@@ -24,19 +24,15 @@
from __future__ import annotations
+from importlib.metadata import PackageNotFoundError, version
-def is_pydantic_2_installed() -> bool:
- import sys
+from pkg_resources import parse_version
Review Comment:
Why do you use `from pkg_resources import parse_version` here and `from
packaging.version import parse as parse_version`? Is that an IDE
auto-suggestion?
##########
airflow/utils/pydantic.py:
##########
@@ -24,19 +24,15 @@
from __future__ import annotations
+from importlib.metadata import PackageNotFoundError, version
Review Comment:
I'm assuming multiple packages provide a `version` function which
potentially does different things. IMHO, it could improve readability if
imported `as version_string` or `as get_version_string`.
##########
airflow/utils/pydantic.py:
##########
@@ -24,19 +24,15 @@
from __future__ import annotations
+from importlib.metadata import PackageNotFoundError, version
-def is_pydantic_2_installed() -> bool:
- import sys
+from pkg_resources import parse_version
- from packaging.version import Version
- if sys.version_info >= (3, 9):
- from importlib.metadata import distribution
- else:
- from importlib_metadata import distribution
+def is_pydantic_2_installed() -> bool:
try:
- return Version(distribution("pydantic").version) >= Version("2.0.0")
- except ImportError:
+ return parse_version(version("pydantic")).major == 2
+ except PackageNotFoundError:
Review Comment:
A unit test for the correct exception type might be an overkill, but I would
add a test just so it reminds us that the string provided to `version` needs to
be the "pypi name" and not the "import name" (i.e. `apache-airflow` would work
whereas `airflow` wouldn't).
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
