Taragolis commented on code in PR #35287:
URL: https://github.com/apache/airflow/pull/35287#discussion_r1377461406
##########
.pre-commit-config.yaml:
##########
@@ -181,13 +174,16 @@ repos:
pass_filenames: false
require_serial: true
- repo: https://github.com/astral-sh/ruff-pre-commit
- rev: v0.0.292
+ rev: v0.1.3
hooks:
# Since ruff makes use of multiple cores we _purposefully_ don't run
this in docker so it can use the
# host CPU to it's fullest
- id: ruff
- name: ruff
- args: [ --fix ]
+ name: ruff-lint
+ args: [--fix]
+ exclude: ^.*/.*_vendor/|^tests/dags/test_imports.py
+ - id: ruff-format
+ name: ruff-format
exclude: ^.*/.*_vendor/|^tests/dags/test_imports.py
Review Comment:
I guess it should be `^.*/.*_vendor/|^airflow/contrib/`
Seems like that is the reason why a lot of changes in to the legacy
`airflow.contrib`
##########
tests/www/views/test_views_base.py:
##########
@@ -88,7 +88,7 @@ def heartbeat_too_slow():
state="running",
latest_heartbeat=last_heartbeat,
)
- SchedulerJobRunner(job=job),
+ (SchedulerJobRunner(job=job),)
Review Comment:
```suggestion
SchedulerJobRunner(job=job)
```
##########
tests/providers/common/sql/operators/test_sql.py:
##########
@@ -131,15 +131,15 @@ class TestColumnCheckOperator:
UNION ALL
SELECT 'X' AS col_name, 'distinct_check' AS check_type,
X_distinct_check AS check_result
FROM (SELECT COUNT(DISTINCT(X)) AS X_distinct_check FROM test_table
WHERE Y > 1) AS sq
- """ # noqa 501
+ """ # 501
Review Comment:
```suggestion
"""
```
##########
tests/system/providers/docker/example_docker.py:
##########
@@ -54,9 +54,7 @@
(
# TEST BODY
- t1
- >> [t2, t3]
- >> t4
+ t1 >> [t2, t3] >> t4
Review Comment:
Personal I don't like this behaviour in brackets. But I guess nothing we
could do here?
##########
airflow/models/dag.py:
##########
@@ -3618,7 +3618,7 @@ def get_paused_dag_ids(dag_ids: list[str], session:
Session = NEW_SESSION) -> se
.where(DagModel.dag_id.in_(dag_ids))
)
- paused_dag_ids = {paused_dag_id for paused_dag_id, in paused_dag_ids}
+ paused_dag_ids = {paused_dag_id for (paused_dag_id,) in paused_dag_ids}
Review Comment:
```suggestion
paused_dag_ids = {paused_dag_id for paused_dag_id in paused_dag_ids}
```
##########
tests/www/views/test_views_base.py:
##########
@@ -68,7 +68,7 @@ def heartbeat_healthy():
state="running",
latest_heartbeat=last_heartbeat,
)
- SchedulerJobRunner(job=job),
+ (SchedulerJobRunner(job=job),)
Review Comment:
Seems like it would be easier to found extra comma in the end, which turned
into tuple.
There is no critical here because we do not assign anything, but better to
fix it
```suggestion
SchedulerJobRunner(job=job)
```
##########
tests/providers/common/sql/operators/test_sql.py:
##########
@@ -131,15 +131,15 @@ class TestColumnCheckOperator:
UNION ALL
SELECT 'X' AS col_name, 'distinct_check' AS check_type,
X_distinct_check AS check_result
FROM (SELECT COUNT(DISTINCT(X)) AS X_distinct_check FROM test_table
WHERE Y > 1) AS sq
- """ # noqa 501
+ """ # 501
correct_generate_sql_query_with_partition_and_where = """
SELECT 'X' AS col_name, 'null_check' AS check_type, X_null_check AS
check_result
FROM (SELECT SUM(CASE WHEN X IS NULL THEN 1 ELSE 0 END) AS
X_null_check FROM test_table WHERE Y > 1 AND Z < 100) AS sq
UNION ALL
SELECT 'X' AS col_name, 'distinct_check' AS check_type,
X_distinct_check AS check_result
FROM (SELECT COUNT(DISTINCT(X)) AS X_distinct_check FROM test_table
WHERE Y > 1) AS sq
- """ # noqa 501
+ """ # 501
Review Comment:
```suggestion
"""
```
##########
tests/providers/google/cloud/hooks/test_datacatalog.py:
##########
@@ -65,9 +65,9 @@
TEST_NAME: str = "test-name"
TEST_TAG_ID: str = "test-tag-id"
TEST_LOCATION_PATH: str = f"projects/{{}}/locations/{TEST_LOCATION}"
-TEST_ENTRY_PATH: str = (
-
f"projects/{{}}/locations/{TEST_LOCATION}/entryGroups/{TEST_ENTRY_GROUP_ID}/entries/{TEST_ENTRY_ID}"
-)
+TEST_ENTRY_PATH: (
+ str
+) =
f"projects/{{}}/locations/{TEST_LOCATION}/entryGroups/{TEST_ENTRY_GROUP_ID}/entries/{TEST_ENTRY_ID}"
Review Comment:
That it also looks less readable rather than `black` formatting
##########
airflow/www/views.py:
##########
@@ -2447,7 +2447,7 @@ def _mark_dagrun_state_as_queued(
)
)
- completed_tis_ids = [task_id for task_id, in existing_tis]
+ completed_tis_ids = [task_id for (task_id,) in existing_tis]
Review Comment:
```suggestion
completed_tis_ids = [task_id for task_id in existing_tis]
```
##########
airflow/api/common/delete_dag.py:
##########
@@ -72,7 +72,7 @@ def delete_dag(dag_id: str, keep_records_in_log: bool = True,
session: Session =
)
)
- dags_to_delete = [dag_id for dag_id, in dags_to_delete_query]
+ dags_to_delete = [dag_id for (dag_id,) in dags_to_delete_query]
Review Comment:
```suggestion
dags_to_delete = [dag_id for dag_id in dags_to_delete_query]
```
##########
airflow/www/views.py:
##########
@@ -929,7 +929,7 @@ def index(self):
dagtags =
session.execute(select(func.distinct(DagTag.name)).order_by(DagTag.name)).all()
tags = [
{"name": name, "selected": bool(arg_tags_filter and name in
arg_tags_filter)}
- for name, in dagtags
+ for (name,) in dagtags
Review Comment:
```suggestion
for name in dagtags
```
##########
airflow/providers/google/cloud/transfers/bigquery_to_mssql.py:
##########
@@ -63,8 +63,7 @@ def __init__(
if mssql_table is not None:
warnings.warn(
# fmt: off
- "The `mssql_table` parameter has been deprecated. "
- "Use `target_table_name` instead.",
+ "The `mssql_table` parameter has been deprecated. Use
`target_table_name` instead.",
# fmt: on
Review Comment:
Not related to this PR but we should found as follow-up all `# fmt: off` and
`# fmt: on` and remove it from codebase. I guess most of them (if not all) not
required
--
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]
