Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check AIR001 from builtin or providers operators module #14631

Merged
merged 1 commit into from
Dec 4, 2024

Conversation

dhruvmanila
Copy link
Member

@dhruvmanila dhruvmanila commented Nov 27, 2024

Summary

This PR makes changes to the AIR001 rule as per #14627 (comment).

Additionally,

  • Avoid returning the Diagnostic and update the checker in the rule logic for consistency
  • Remove test case for different keyword position (I don't think it's required here)

Test Plan

Add test cases for multiple operators from various modules.

Copy link
Contributor

ruff-ecosystem results

Linter (stable)

ℹ️ ecosystem check detected linter changes. (+0 -678 violations, +0 -0 fixes in 1 projects; 54 projects unchanged)

apache/airflow (+0 -678 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --no-preview --select ALL

- airflow/example_dags/example_dynamic_task_mapping_with_no_taskflow_operators.py:64:5: AIR001 Task variable name should match the `task_id`: "sum_it"
- airflow/example_dags/example_sensors.py:102:5: AIR001 Task variable name should match the `task_id`: "wait_for_file_async"
- airflow/example_dags/example_sensors.py:112:5: AIR001 Task variable name should match the `task_id`: "success_sensor_python"
- airflow/example_dags/example_sensors.py:114:5: AIR001 Task variable name should match the `task_id`: "failure_timeout_sensor_python"
- airflow/example_dags/example_sensors.py:120:5: AIR001 Task variable name should match the `task_id`: "week_day_sensor_failing_on_timeout"
- airflow/example_dags/example_sensors.py:56:5: AIR001 Task variable name should match the `task_id`: "wait_some_seconds"
- airflow/example_dags/example_sensors.py:60:5: AIR001 Task variable name should match the `task_id`: "wait_some_seconds_async"
- airflow/example_dags/example_sensors.py:64:5: AIR001 Task variable name should match the `task_id`: "fire_immediately"
- airflow/example_dags/example_sensors.py:68:5: AIR001 Task variable name should match the `task_id`: "timeout_after_second_date_in_the_future"
- airflow/example_dags/example_sensors.py:77:5: AIR001 Task variable name should match the `task_id`: "fire_immediately_async"
- airflow/example_dags/example_sensors.py:81:5: AIR001 Task variable name should match the `task_id`: "timeout_after_second_date_in_the_future_async"
- airflow/example_dags/example_sensors.py:90:5: AIR001 Task variable name should match the `task_id`: "Sensor_succeeds"
- airflow/example_dags/example_sensors.py:92:5: AIR001 Task variable name should match the `task_id`: "Sensor_fails_after_3_seconds"
- airflow/example_dags/example_sensors.py:98:5: AIR001 Task variable name should match the `task_id`: "wait_for_file"
- providers/src/airflow/providers/arangodb/example_dags/example_arangodb.py:34:1: AIR001 Task variable name should match the `task_id`: "aql_sensor"
- providers/src/airflow/providers/arangodb/example_dags/example_arangodb.py:46:1: AIR001 Task variable name should match the `task_id`: "aql_sensor_template_file"
- providers/src/airflow/providers/google/cloud/example_dags/example_cloud_task.py:48:5: AIR001 Task variable name should match the `task_id`: "gcp_sense_cloud_tasks_empty"
- providers/src/airflow/providers/google/cloud/example_dags/example_facebook_ads_to_gcs.py:102:5: AIR001 Task variable name should match the `task_id`: "gcs_to_bq_example"
- providers/src/airflow/providers/google/cloud/example_dags/example_facebook_ads_to_gcs.py:91:5: AIR001 Task variable name should match the `task_id`: "run_fetch_data"
- providers/src/airflow/providers/google/cloud/example_dags/example_salesforce_to_gcs.py:60:5: AIR001 Task variable name should match the `task_id`: "upload_to_gcs"
- providers/src/airflow/providers/google/cloud/example_dags/example_salesforce_to_gcs.py:95:5: AIR001 Task variable name should match the `task_id`: "gcs_to_bq"
- providers/tests/amazon/aws/sensors/test_batch.py:111:9: AIR001 Task variable name should match the `task_id`: "batch_job_sensor"
- providers/tests/amazon/aws/sensors/test_cloud_formation.py:108:9: AIR001 Task variable name should match the `task_id`: "cf_delete_stack_init"
- providers/tests/amazon/aws/sensors/test_cloud_formation.py:119:9: AIR001 Task variable name should match the `task_id`: "task"
- providers/tests/amazon/aws/sensors/test_cloud_formation.py:124:9: AIR001 Task variable name should match the `task_id`: "task"
- providers/tests/amazon/aws/sensors/test_cloud_formation.py:129:9: AIR001 Task variable name should match the `task_id`: "task"
- providers/tests/amazon/aws/sensors/test_cloud_formation.py:135:9: AIR001 Task variable name should match the `task_id`: "task"
- providers/tests/amazon/aws/sensors/test_cloud_formation.py:44:9: AIR001 Task variable name should match the `task_id`: "cf_create_stack_init"
- providers/tests/amazon/aws/sensors/test_cloud_formation.py:61:9: AIR001 Task variable name should match the `task_id`: "cf_create_stack_init"
- providers/tests/amazon/aws/sensors/test_cloud_formation.py:70:9: AIR001 Task variable name should match the `task_id`: "task"
- providers/tests/amazon/aws/sensors/test_cloud_formation.py:75:9: AIR001 Task variable name should match the `task_id`: "task"
- providers/tests/amazon/aws/sensors/test_cloud_formation.py:80:9: AIR001 Task variable name should match the `task_id`: "task"
- providers/tests/amazon/aws/sensors/test_cloud_formation.py:91:9: AIR001 Task variable name should match the `task_id`: "cf_delete_stack_init"
- providers/tests/amazon/aws/sensors/test_dms.py:61:9: AIR001 Task variable name should match the `task_id`: "create_task"
- providers/tests/amazon/aws/sensors/test_dynamodb.py:155:9: AIR001 Task variable name should match the `task_id`: "dynamodb_value_sensor_init"
- providers/tests/amazon/aws/sensors/test_dynamodb.py:178:9: AIR001 Task variable name should match the `task_id`: "dynamodb_value_sensor_init"
- providers/tests/amazon/aws/sensors/test_ec2.py:30:9: AIR001 Task variable name should match the `task_id`: "task_test"
- providers/tests/amazon/aws/sensors/test_ecs.py:84:9: AIR001 Task variable name should match the `task_id`: "test_ecs_base"
- providers/tests/amazon/aws/sensors/test_ecs.py:95:9: AIR001 Task variable name should match the `task_id`: "test_ecs_base_hook_client"
- providers/tests/amazon/aws/sensors/test_emr_job_flow.py:207:9: AIR001 Task variable name should match the `task_id`: "test_task"
- providers/tests/amazon/aws/sensors/test_emr_job_flow.py:226:9: AIR001 Task variable name should match the `task_id`: "test_task"
- providers/tests/amazon/aws/sensors/test_emr_job_flow.py:248:9: AIR001 Task variable name should match the `task_id`: "test_task"
- providers/tests/amazon/aws/sensors/test_emr_job_flow.py:265:9: AIR001 Task variable name should match the `task_id`: "test_task"
- providers/tests/amazon/aws/sensors/test_emr_notebook_execution.py:44:9: AIR001 Task variable name should match the `task_id`: "test_task"
- providers/tests/amazon/aws/sensors/test_emr_notebook_execution.py:56:9: AIR001 Task variable name should match the `task_id`: "test_task"
- providers/tests/amazon/aws/sensors/test_emr_notebook_execution.py:70:9: AIR001 Task variable name should match the `task_id`: "test_task"
- providers/tests/amazon/aws/sensors/test_emr_step.py:205:9: AIR001 Task variable name should match the `task_id`: "test_task"
- providers/tests/amazon/aws/sensors/test_glue.py:120:9: AIR001 Task variable name should match the `task_id`: "test_glue_job_sensor"
- providers/tests/amazon/aws/sensors/test_glue.py:141:9: AIR001 Task variable name should match the `task_id`: "test_glue_job_sensor"
- providers/tests/amazon/aws/sensors/test_glue.py:36:9: AIR001 Task variable name should match the `task_id`: "test_glue_job_sensor"
... 628 additional changes omitted for project

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
AIR001 678 0 678 0 0

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+0 -678 violations, +0 -0 fixes in 1 projects; 54 projects unchanged)

apache/airflow (+0 -678 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

- airflow/example_dags/example_dynamic_task_mapping_with_no_taskflow_operators.py:64:5: AIR001 Task variable name should match the `task_id`: "sum_it"
- airflow/example_dags/example_sensors.py:102:5: AIR001 Task variable name should match the `task_id`: "wait_for_file_async"
- airflow/example_dags/example_sensors.py:112:5: AIR001 Task variable name should match the `task_id`: "success_sensor_python"
- airflow/example_dags/example_sensors.py:114:5: AIR001 Task variable name should match the `task_id`: "failure_timeout_sensor_python"
- airflow/example_dags/example_sensors.py:120:5: AIR001 Task variable name should match the `task_id`: "week_day_sensor_failing_on_timeout"
- airflow/example_dags/example_sensors.py:56:5: AIR001 Task variable name should match the `task_id`: "wait_some_seconds"
- airflow/example_dags/example_sensors.py:60:5: AIR001 Task variable name should match the `task_id`: "wait_some_seconds_async"
- airflow/example_dags/example_sensors.py:64:5: AIR001 Task variable name should match the `task_id`: "fire_immediately"
- airflow/example_dags/example_sensors.py:68:5: AIR001 Task variable name should match the `task_id`: "timeout_after_second_date_in_the_future"
- airflow/example_dags/example_sensors.py:77:5: AIR001 Task variable name should match the `task_id`: "fire_immediately_async"
- airflow/example_dags/example_sensors.py:81:5: AIR001 Task variable name should match the `task_id`: "timeout_after_second_date_in_the_future_async"
- airflow/example_dags/example_sensors.py:90:5: AIR001 Task variable name should match the `task_id`: "Sensor_succeeds"
- airflow/example_dags/example_sensors.py:92:5: AIR001 Task variable name should match the `task_id`: "Sensor_fails_after_3_seconds"
- airflow/example_dags/example_sensors.py:98:5: AIR001 Task variable name should match the `task_id`: "wait_for_file"
- providers/src/airflow/providers/arangodb/example_dags/example_arangodb.py:34:1: AIR001 Task variable name should match the `task_id`: "aql_sensor"
- providers/src/airflow/providers/arangodb/example_dags/example_arangodb.py:46:1: AIR001 Task variable name should match the `task_id`: "aql_sensor_template_file"
- providers/src/airflow/providers/google/cloud/example_dags/example_cloud_task.py:48:5: AIR001 Task variable name should match the `task_id`: "gcp_sense_cloud_tasks_empty"
- providers/src/airflow/providers/google/cloud/example_dags/example_facebook_ads_to_gcs.py:102:5: AIR001 Task variable name should match the `task_id`: "gcs_to_bq_example"
- providers/src/airflow/providers/google/cloud/example_dags/example_facebook_ads_to_gcs.py:91:5: AIR001 Task variable name should match the `task_id`: "run_fetch_data"
- providers/src/airflow/providers/google/cloud/example_dags/example_salesforce_to_gcs.py:60:5: AIR001 Task variable name should match the `task_id`: "upload_to_gcs"
- providers/src/airflow/providers/google/cloud/example_dags/example_salesforce_to_gcs.py:95:5: AIR001 Task variable name should match the `task_id`: "gcs_to_bq"
- providers/tests/amazon/aws/sensors/test_batch.py:111:9: AIR001 Task variable name should match the `task_id`: "batch_job_sensor"
- providers/tests/amazon/aws/sensors/test_cloud_formation.py:108:9: AIR001 Task variable name should match the `task_id`: "cf_delete_stack_init"
- providers/tests/amazon/aws/sensors/test_cloud_formation.py:119:9: AIR001 Task variable name should match the `task_id`: "task"
- providers/tests/amazon/aws/sensors/test_cloud_formation.py:124:9: AIR001 Task variable name should match the `task_id`: "task"
- providers/tests/amazon/aws/sensors/test_cloud_formation.py:129:9: AIR001 Task variable name should match the `task_id`: "task"
- providers/tests/amazon/aws/sensors/test_cloud_formation.py:135:9: AIR001 Task variable name should match the `task_id`: "task"
- providers/tests/amazon/aws/sensors/test_cloud_formation.py:44:9: AIR001 Task variable name should match the `task_id`: "cf_create_stack_init"
- providers/tests/amazon/aws/sensors/test_cloud_formation.py:61:9: AIR001 Task variable name should match the `task_id`: "cf_create_stack_init"
- providers/tests/amazon/aws/sensors/test_cloud_formation.py:70:9: AIR001 Task variable name should match the `task_id`: "task"
- providers/tests/amazon/aws/sensors/test_cloud_formation.py:75:9: AIR001 Task variable name should match the `task_id`: "task"
- providers/tests/amazon/aws/sensors/test_cloud_formation.py:80:9: AIR001 Task variable name should match the `task_id`: "task"
- providers/tests/amazon/aws/sensors/test_cloud_formation.py:91:9: AIR001 Task variable name should match the `task_id`: "cf_delete_stack_init"
- providers/tests/amazon/aws/sensors/test_dms.py:61:9: AIR001 Task variable name should match the `task_id`: "create_task"
- providers/tests/amazon/aws/sensors/test_dynamodb.py:155:9: AIR001 Task variable name should match the `task_id`: "dynamodb_value_sensor_init"
- providers/tests/amazon/aws/sensors/test_dynamodb.py:178:9: AIR001 Task variable name should match the `task_id`: "dynamodb_value_sensor_init"
- providers/tests/amazon/aws/sensors/test_ec2.py:30:9: AIR001 Task variable name should match the `task_id`: "task_test"
- providers/tests/amazon/aws/sensors/test_ecs.py:84:9: AIR001 Task variable name should match the `task_id`: "test_ecs_base"
- providers/tests/amazon/aws/sensors/test_ecs.py:95:9: AIR001 Task variable name should match the `task_id`: "test_ecs_base_hook_client"
- providers/tests/amazon/aws/sensors/test_emr_job_flow.py:207:9: AIR001 Task variable name should match the `task_id`: "test_task"
- providers/tests/amazon/aws/sensors/test_emr_job_flow.py:226:9: AIR001 Task variable name should match the `task_id`: "test_task"
- providers/tests/amazon/aws/sensors/test_emr_job_flow.py:248:9: AIR001 Task variable name should match the `task_id`: "test_task"
- providers/tests/amazon/aws/sensors/test_emr_job_flow.py:265:9: AIR001 Task variable name should match the `task_id`: "test_task"
- providers/tests/amazon/aws/sensors/test_emr_notebook_execution.py:44:9: AIR001 Task variable name should match the `task_id`: "test_task"
- providers/tests/amazon/aws/sensors/test_emr_notebook_execution.py:56:9: AIR001 Task variable name should match the `task_id`: "test_task"
- providers/tests/amazon/aws/sensors/test_emr_notebook_execution.py:70:9: AIR001 Task variable name should match the `task_id`: "test_task"
- providers/tests/amazon/aws/sensors/test_emr_step.py:205:9: AIR001 Task variable name should match the `task_id`: "test_task"
- providers/tests/amazon/aws/sensors/test_glue.py:120:9: AIR001 Task variable name should match the `task_id`: "test_glue_job_sensor"
- providers/tests/amazon/aws/sensors/test_glue.py:141:9: AIR001 Task variable name should match the `task_id`: "test_glue_job_sensor"
- providers/tests/amazon/aws/sensors/test_glue.py:36:9: AIR001 Task variable name should match the `task_id`: "test_glue_job_sensor"
... 628 additional changes omitted for project

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
AIR001 678 0 678 0 0

@MichaReiser
Copy link
Member

CC: @uranusjr

@MichaReiser
Copy link
Member

That's a lot of removed diagnostics

@MichaReiser MichaReiser added the rule Implementing or modifying a lint rule label Nov 27, 2024
@dhruvmanila
Copy link
Member Author

dhruvmanila commented Nov 27, 2024

That's a lot of removed diagnostics

That's because I've removed a couple of test cases which just moves the keyword argument through different position. I don't think the rule should test that.

Oops, you meant the ecosystem changes and not the snapshot changes.

Comment on lines 10 to -9
my_task = PythonOperator(task_id="my_task", callable=my_callable)
my_task_2 = PythonOperator(callable=my_callable, task_id="my_task_2")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @MichaReiser these two test cases are identical except that the arguments are reversed.

@MichaReiser
Copy link
Member

Yeah. I'd like for @uranusjr to have a look at the ecosystem changes. I worry that we just removed many true-positives, because Ruff can't see through other import/classes.

@MichaReiser
Copy link
Member

@uranusjr sorry for pinging you again. Would you mind taking a look at the ecosystem changes? I want to make sure that we don't regress an airflow rule.

@MichaReiser MichaReiser mentioned this pull request Dec 4, 2024
2 tasks
@MichaReiser
Copy link
Member

See #14626 (comment)

@dhruvmanila dhruvmanila merged commit 575deb5 into main Dec 4, 2024
21 checks passed
@dhruvmanila dhruvmanila deleted the dhruv/air001 branch December 4, 2024 08:00
@uranusjr
Copy link
Contributor

uranusjr commented Dec 4, 2024

I’m late, but this looks good to me as well. Awesome work.

dcreager added a commit that referenced this pull request Dec 4, 2024
* main:
  [red-knot] Test: Hashable/Sized => A/B (#14769)
  [`flake8-type-checking`] Expands TC006 docs to better explain itself (#14749)
  [`pycodestyle`] Handle f-strings properly for `invalid-escape-sequence (W605)` (#14748)
  [red-knot] Add fuzzer to catch panics for invalid syntax (#14678)
  Check `AIR001` from builtin or providers `operators` module (#14631)
  [airflow]: extend removed names (AIR302) (#14734)
@dhruvmanila
Copy link
Member Author

I’m late, but this looks good to me as well. Awesome work.

No worries, thanks for looking into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants