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/11/30 12:00:54 UTC
[GitHub] [airflow] potiuk opened a new pull request #19890: Re-enables MyPy in non-failure mode
potiuk opened a new pull request #19890:
URL: https://github.com/apache/airflow/pull/19890
This PR enables mypy back as pre-commit for local changes after
the #19317 switched to Python 3.7 but also it separates out
mypy to a separate non-failing step in CI.
In the CI we will be able to see remaining mypy errors.
This will allow us to gradually fix all the mypy errors and enable
mypy back when we got all the problems fixed.
<!--
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] github-actions[bot] commented on pull request #19890: Re-enables MyPy in non-failure mode
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #19890:
URL: https://github.com/apache/airflow/pull/19890#issuecomment-983110712
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 commented on pull request #19890: Re-enables MyPy in non-failure mode
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #19890:
URL: https://github.com/apache/airflow/pull/19890#issuecomment-982678933
I also did not commit some workflow modifications from a bigger commit - this one should be much better.
--
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 #19890: Re-enables MyPy in non-failure mode
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #19890:
URL: https://github.com/apache/airflow/pull/19890#issuecomment-983026758
Looks GOOD.
--
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 #19890: Re-enables MyPy in non-failure mode
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #19890:
URL: https://github.com/apache/airflow/pull/19890#discussion_r759250354
##########
File path: scripts/in_container/run_mypy.sh
##########
@@ -20,18 +20,5 @@
. "$( dirname "${BASH_SOURCE[0]}" )/_in_container_script_init.sh"
export PYTHONPATH=${AIRFLOW_SOURCES}
-mypy_args=()
-
-# Mypy doesn't cope very well with namespace packages when give filenames (it gets confused about the lack of __init__.py in airflow.providers, and thinks airflow.providers.docker is the same as a "docker" top level module).
-#
-# So we instead need to convert the file names in to module names to check
-for filename in "$@";
-do
- if [[ "${filename}" == docs/* ]]; then
- mypy_args+=("$filename")
- else
- mypy_args+=("-m" "$(filename_to_python_module "$filename")")
- fi
-done
-
-mypy --namespace-packages "${mypy_args[@]}"
+# Seems that new mypy copes better with namespace packages
Review comment:
```suggestion
```
--
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 #19890: Re-enables MyPy in non-failure mode
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #19890:
URL: https://github.com/apache/airflow/pull/19890#issuecomment-982958759
I think this one should be ready to merge, we need to merge it to start working on #19891
--
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 #19890: Re-enables MyPy in non-failure mode
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #19890:
URL: https://github.com/apache/airflow/pull/19890#discussion_r759311644
##########
File path: setup.py
##########
@@ -893,6 +917,10 @@ def get_all_provider_packages() -> str:
class AirflowDistribution(Distribution):
"""The setuptools.Distribution subclass with Airflow specific behaviour"""
+ def __init__(self, attrs=None):
+ super().__init__(attrs)
+ self.install_requires = None
Review comment:
Fixing mypy error (not initialized `install_requires`). MyPy complains when local pre-commit is run.
--
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 #19890: Re-enables MyPy in non-failure mode
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #19890:
URL: https://github.com/apache/airflow/pull/19890#discussion_r759251996
##########
File path: setup.py
##########
@@ -893,6 +917,10 @@ def get_all_provider_packages() -> str:
class AirflowDistribution(Distribution):
"""The setuptools.Distribution subclass with Airflow specific behaviour"""
+ def __init__(self, attrs=None):
+ super().__init__(attrs)
+ self.install_requires = None
Review comment:
What's this for?
--
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 merged pull request #19890: Re-enables MyPy in non-failure mode
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #19890:
URL: https://github.com/apache/airflow/pull/19890
--
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 #19890: Re-enables MyPy in non-failure mode
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #19890:
URL: https://github.com/apache/airflow/pull/19890#discussion_r759251677
##########
File path: setup.py
##########
@@ -513,6 +513,31 @@ def write_version(filename: str = os.path.join(*[my_dir, "airflow", "git_version
]
# End dependencies group
+# Mypy 0.900 and above ships only with stubs from stdlib so if we need other stubs, we need to install them
+# manually as `types-*`. See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
+# for details. Wy want to install them explicitly because we want to eventually move to
+# mypyd which does not support installing the types dynamically with --install-types
+mypy_dependencies = [
+ 'mypy',
Review comment:
Should we pin/restrict the mypy version? I think we had problems in the past where mypy would update and change behaviour in a few places.
--
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 #19890: Re-enables MyPy in non-failure mode
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #19890:
URL: https://github.com/apache/airflow/pull/19890#discussion_r759254685
##########
File path: setup.py
##########
@@ -513,6 +513,31 @@ def write_version(filename: str = os.path.join(*[my_dir, "airflow", "git_version
]
# End dependencies group
+# Mypy 0.900 and above ships only with stubs from stdlib so if we need other stubs, we need to install them
+# manually as `types-*`. See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
+# for details. Wy want to install them explicitly because we want to eventually move to
+# mypyd which does not support installing the types dynamically with --install-types
+mypy_dependencies = [
+ 'mypy',
Review comment:
Yes. I think I lost it when rebasing.
--
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