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/12/22 16:51:07 UTC
[GitHub] [airflow] potiuk opened a new pull request #20466: Add autoflake precommit to automatically remove unused code
potiuk opened a new pull request #20466:
URL: https://github.com/apache/airflow/pull/20466
<!--
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 #20466: Add autoflake precommit to automatically remove unused code
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #20466:
URL: https://github.com/apache/airflow/pull/20466#issuecomment-1000275532
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] eladkal commented on a change in pull request #20466: Add autoflake precommit to automatically remove unused code
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #20466:
URL: https://github.com/apache/airflow/pull/20466#discussion_r774172072
##########
File path: airflow/www/views.py
##########
@@ -4877,22 +4877,14 @@ class CustomUserDBModelView(MultiResourceUserMixin, UserDBModelView):
class CustomUserLDAPModelView(MultiResourceUserMixin, UserLDAPModelView):
"""Customize permission names for FAB's builtin UserLDAPModelView."""
- pass
-
Review comment:
~~This would be resulted in EOF while parsing
isn't it?~~
ahh the comment makes it ok :)
--
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 #20466: Add autoflake precommit to automatically remove unused code
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #20466:
URL: https://github.com/apache/airflow/pull/20466#discussion_r774043884
##########
File path: airflow/models/__init__.py
##########
@@ -16,24 +16,24 @@
# specific language governing permissions and limitations
# under the License.
"""Airflow models"""
-from airflow.models.base import ID_LEN, Base
-from airflow.models.baseoperator import BaseOperator, BaseOperatorLink
-from airflow.models.connection import Connection
-from airflow.models.dag import DAG, DagModel, DagTag
-from airflow.models.dagbag import DagBag
-from airflow.models.dagpickle import DagPickle
-from airflow.models.dagrun import DagRun
-from airflow.models.errors import ImportError
-from airflow.models.log import Log
-from airflow.models.param import Param
-from airflow.models.pool import Pool
-from airflow.models.renderedtifields import RenderedTaskInstanceFields
-from airflow.models.sensorinstance import SensorInstance # noqa: F401
-from airflow.models.skipmixin import SkipMixin
-from airflow.models.slamiss import SlaMiss
-from airflow.models.taskfail import TaskFail
-from airflow.models.taskinstance import TaskInstance, clear_task_instances
-from airflow.models.taskreschedule import TaskReschedule
-from airflow.models.trigger import Trigger
-from airflow.models.variable import Variable
-from airflow.models.xcom import XCOM_RETURN_KEY, XCom
+from airflow.models.base import ID_LEN, Base # noqa: autoflake
Review comment:
I could not find a way to do it. Moving # noqa: to the top of the file does not work and their doc only mention "line" exclusion
--
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] jedcunningham commented on a change in pull request #20466: Add autoflake precommit to automatically remove unused code
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #20466:
URL: https://github.com/apache/airflow/pull/20466#discussion_r774036443
##########
File path: airflow/models/__init__.py
##########
@@ -16,24 +16,24 @@
# specific language governing permissions and limitations
# under the License.
"""Airflow models"""
-from airflow.models.base import ID_LEN, Base
-from airflow.models.baseoperator import BaseOperator, BaseOperatorLink
-from airflow.models.connection import Connection
-from airflow.models.dag import DAG, DagModel, DagTag
-from airflow.models.dagbag import DagBag
-from airflow.models.dagpickle import DagPickle
-from airflow.models.dagrun import DagRun
-from airflow.models.errors import ImportError
-from airflow.models.log import Log
-from airflow.models.param import Param
-from airflow.models.pool import Pool
-from airflow.models.renderedtifields import RenderedTaskInstanceFields
-from airflow.models.sensorinstance import SensorInstance # noqa: F401
-from airflow.models.skipmixin import SkipMixin
-from airflow.models.slamiss import SlaMiss
-from airflow.models.taskfail import TaskFail
-from airflow.models.taskinstance import TaskInstance, clear_task_instances
-from airflow.models.taskreschedule import TaskReschedule
-from airflow.models.trigger import Trigger
-from airflow.models.variable import Variable
-from airflow.models.xcom import XCOM_RETURN_KEY, XCom
+from airflow.models.base import ID_LEN, Base # noqa: autoflake
Review comment:
Can we ignore this whole file instead of doing it on every line?
##########
File path: airflow/kubernetes/pod_runtime_info_env.py
##########
@@ -16,12 +16,10 @@
# specific language governing permissions and limitations
# under the License.
"""This module is deprecated. Please use :mod:`kubernetes.client.models.V1EnvVar`."""
-# flake8: noqa
-
import warnings
with warnings.catch_warnings():
- from airflow.providers.cncf.kubernetes.backcompat.pod_runtime_info_env import PodRuntimeInfoEnv
+ from airflow.providers.cncf.kubernetes.backcompat.pod_runtime_info_env import PodRuntimeInfoEnv # noqa
Review comment:
```suggestion
from airflow.providers.cncf.kubernetes.backcompat.pod_runtime_info_env import PodRuntimeInfoEnv # noqa: autoflake
```
Did you leave that off due to line length?
--
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 #20466: Add autoflake precommit to automatically remove unused code
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #20466:
URL: https://github.com/apache/airflow/pull/20466#issuecomment-999887001
> It's not the line of code that worries me but the no replies to the issues & PRs. But yeah not a big deal we can remove it if when it breaks or one of us can fork it or help maintain it
Yeah. I saw it too. And good you checked. Maybe not "responsive" enough but I think most of those issues are for "pre-python 3" and some very specific requests.
And we can always remove it if we see problems - luckily it's just "when we make commit" type of dependency - we don't even add it to airflow's setup.* files :).
The main reason for me to add it is that I plan to add "execute(context: "Context") all over the providers automatically and that results in some manual 'remove imports" - but I figured it could also helped in the past where I had to manually remove some imports but pre-commits could do it for me :)
Let's try and see - and we can always remove it if we find it causing problems.
--
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 #20466: Add autoflake precommit to automatically remove unused code
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #20466:
URL: https://github.com/apache/airflow/pull/20466#issuecomment-999742619
Its literally 1000 lines of code https://github.com/myint/autoflake/blob/master/autoflake.py @kaxil (and it's not upper bound with pyflakes version): https://github.com/PyCQA/pyflakes - Nov 21 2021 last modification.
--
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 pull request #20466: Add autoflake precommit to automatically remove unused code
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #20466:
URL: https://github.com/apache/airflow/pull/20466#issuecomment-999858476
> Its literally 1000 lines of code https://github.com/myint/autoflake/blob/master/autoflake.py @kaxil (and it's not upper bound with pyflakes version): https://github.com/PyCQA/pyflakes - Nov 21 2021 last modification.
It's not the line of code that worries me but the no replies to the issues & PRs. But yeah not a big deal we can remove it if when it breaks or one of us can fork it or help maintain 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] potiuk commented on a change in pull request #20466: Add autoflake precommit to automatically remove unused code
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #20466:
URL: https://github.com/apache/airflow/pull/20466#discussion_r774181798
##########
File path: airflow/www/views.py
##########
@@ -4877,22 +4877,14 @@ class CustomUserDBModelView(MultiResourceUserMixin, UserDBModelView):
class CustomUserLDAPModelView(MultiResourceUserMixin, UserLDAPModelView):
"""Customize permission names for FAB's builtin UserLDAPModelView."""
- pass
-
Review comment:
Yep :)
--
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 #20466: Add autoflake precommit to automatically remove unused code
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #20466:
URL: https://github.com/apache/airflow/pull/20466#discussion_r774048411
##########
File path: airflow/models/__init__.py
##########
@@ -16,24 +16,24 @@
# specific language governing permissions and limitations
# under the License.
"""Airflow models"""
-from airflow.models.base import ID_LEN, Base
-from airflow.models.baseoperator import BaseOperator, BaseOperatorLink
-from airflow.models.connection import Connection
-from airflow.models.dag import DAG, DagModel, DagTag
-from airflow.models.dagbag import DagBag
-from airflow.models.dagpickle import DagPickle
-from airflow.models.dagrun import DagRun
-from airflow.models.errors import ImportError
-from airflow.models.log import Log
-from airflow.models.param import Param
-from airflow.models.pool import Pool
-from airflow.models.renderedtifields import RenderedTaskInstanceFields
-from airflow.models.sensorinstance import SensorInstance # noqa: F401
-from airflow.models.skipmixin import SkipMixin
-from airflow.models.slamiss import SlaMiss
-from airflow.models.taskfail import TaskFail
-from airflow.models.taskinstance import TaskInstance, clear_task_instances
-from airflow.models.taskreschedule import TaskReschedule
-from airflow.models.trigger import Trigger
-from airflow.models.variable import Variable
-from airflow.models.xcom import XCOM_RETURN_KEY, XCom
+from airflow.models.base import ID_LEN, Base # noqa: autoflake
Review comment:
WAY better now.
--
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 #20466: Add autoflake precommit to automatically remove unused code
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #20466:
URL: https://github.com/apache/airflow/pull/20466#discussion_r774046541
##########
File path: airflow/models/__init__.py
##########
@@ -16,24 +16,24 @@
# specific language governing permissions and limitations
# under the License.
"""Airflow models"""
-from airflow.models.base import ID_LEN, Base
-from airflow.models.baseoperator import BaseOperator, BaseOperatorLink
-from airflow.models.connection import Connection
-from airflow.models.dag import DAG, DagModel, DagTag
-from airflow.models.dagbag import DagBag
-from airflow.models.dagpickle import DagPickle
-from airflow.models.dagrun import DagRun
-from airflow.models.errors import ImportError
-from airflow.models.log import Log
-from airflow.models.param import Param
-from airflow.models.pool import Pool
-from airflow.models.renderedtifields import RenderedTaskInstanceFields
-from airflow.models.sensorinstance import SensorInstance # noqa: F401
-from airflow.models.skipmixin import SkipMixin
-from airflow.models.slamiss import SlaMiss
-from airflow.models.taskfail import TaskFail
-from airflow.models.taskinstance import TaskInstance, clear_task_instances
-from airflow.models.taskreschedule import TaskReschedule
-from airflow.models.trigger import Trigger
-from airflow.models.variable import Variable
-from airflow.models.xcom import XCOM_RETURN_KEY, XCom
+from airflow.models.base import ID_LEN, Base # noqa: autoflake
Review comment:
Autoflake has : --ignore-init-module-imports which is generally what we want !
--
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 #20466: Add autoflake precommit to automatically remove unused code
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #20466:
URL: https://github.com/apache/airflow/pull/20466#discussion_r774040874
##########
File path: airflow/kubernetes/pod_runtime_info_env.py
##########
@@ -16,12 +16,10 @@
# specific language governing permissions and limitations
# under the License.
"""This module is deprecated. Please use :mod:`kubernetes.client.models.V1EnvVar`."""
-# flake8: noqa
-
import warnings
with warnings.catch_warnings():
- from airflow.providers.cncf.kubernetes.backcompat.pod_runtime_info_env import PodRuntimeInfoEnv
+ from airflow.providers.cncf.kubernetes.backcompat.pod_runtime_info_env import PodRuntimeInfoEnv # noqa
Review comment:
yes
--
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] eladkal commented on a change in pull request #20466: Add autoflake precommit to automatically remove unused code
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #20466:
URL: https://github.com/apache/airflow/pull/20466#discussion_r774172072
##########
File path: airflow/www/views.py
##########
@@ -4877,22 +4877,14 @@ class CustomUserDBModelView(MultiResourceUserMixin, UserDBModelView):
class CustomUserLDAPModelView(MultiResourceUserMixin, UserLDAPModelView):
"""Customize permission names for FAB's builtin UserLDAPModelView."""
- pass
-
Review comment:
This would be resulted in EOF while parsing
isn't 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] potiuk commented on pull request #20466: Add autoflake precommit to automatically remove unused code
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #20466:
URL: https://github.com/apache/airflow/pull/20466#issuecomment-999740922
> Looks good but my fear is that the library autoflake isn't maintained actively - no responses on issues - https://github.com/myint/autoflake/issues and last commit was in Dec 2020
Autoflake uses pyflakes under the hood - same as flake8. It's really a very small "remove stuff that pyflakes reports". I think this is the reason it's not modified.
--
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 #20466: Add autoflake precommit to automatically remove unused code
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #20466:
URL: https://github.com/apache/airflow/pull/20466
--
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 closed pull request #20466: Add autoflake precommit to automatically remove unused code
Posted by GitBox <gi...@apache.org>.
potiuk closed pull request #20466:
URL: https://github.com/apache/airflow/pull/20466
--
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 #20466: Add autoflake precommit to automatically remove unused code
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #20466:
URL: https://github.com/apache/airflow/pull/20466#discussion_r774045730
##########
File path: airflow/models/__init__.py
##########
@@ -16,24 +16,24 @@
# specific language governing permissions and limitations
# under the License.
"""Airflow models"""
-from airflow.models.base import ID_LEN, Base
-from airflow.models.baseoperator import BaseOperator, BaseOperatorLink
-from airflow.models.connection import Connection
-from airflow.models.dag import DAG, DagModel, DagTag
-from airflow.models.dagbag import DagBag
-from airflow.models.dagpickle import DagPickle
-from airflow.models.dagrun import DagRun
-from airflow.models.errors import ImportError
-from airflow.models.log import Log
-from airflow.models.param import Param
-from airflow.models.pool import Pool
-from airflow.models.renderedtifields import RenderedTaskInstanceFields
-from airflow.models.sensorinstance import SensorInstance # noqa: F401
-from airflow.models.skipmixin import SkipMixin
-from airflow.models.slamiss import SlaMiss
-from airflow.models.taskfail import TaskFail
-from airflow.models.taskinstance import TaskInstance, clear_task_instances
-from airflow.models.taskreschedule import TaskReschedule
-from airflow.models.trigger import Trigger
-from airflow.models.variable import Variable
-from airflow.models.xcom import XCOM_RETURN_KEY, XCom
+from airflow.models.base import ID_LEN, Base # noqa: autoflake
Review comment:
Ahd. Found a better way !
--
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 #20466: Add autoflake precommit to automatically remove unused code
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #20466:
URL: https://github.com/apache/airflow/pull/20466#issuecomment-999722401
I'v got enough of manually removing unused imports :)
--
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