amoghrajesh commented on code in PR #53791:
URL: https://github.com/apache/airflow/pull/53791#discussion_r2238537798
##########
devel-common/src/tests_common/test_utils/mock_operators.py:
##########
@@ -34,6 +32,11 @@
else:
from airflow.models.xcom import XCom # type: ignore[no-redef]
+if AIRFLOW_V_3_1_PLUS:
+ from airflow.sdk.bases.operator import BaseOperator
+else:
+ from airflow.models.baseoperator import BaseOperator # type:
ignore[no-redef]
Review Comment:
Same here, let's use try / except.
##########
providers/standard/src/airflow/providers/standard/operators/hitl.py:
##########
@@ -29,10 +29,10 @@
from datetime import datetime, timezone
from typing import TYPE_CHECKING, Any
-from airflow.models.baseoperator import BaseOperator
from airflow.providers.standard.exceptions import HITLTimeoutError,
HITLTriggerEventError
from airflow.providers.standard.triggers.hitl import HITLTrigger,
HITLTriggerEventSuccessPayload
from airflow.providers.standard.utils.skipmixin import SkipMixin
+from airflow.sdk.bases.operator import BaseOperator
Review Comment:
This could break compat, you should add it with a try / except here.
Unless the operator is only supposed to work for 3.1+
cc @Lee-W
##########
providers/standard/src/airflow/providers/standard/sensors/time.py:
##########
@@ -42,7 +42,10 @@ class StartTriggerArgs: # type: ignore[no-redef]
timeout: datetime.timedelta | None = None
-from airflow.utils import timezone
+try:
+ from airflow.sdk import timezone
Review Comment:
This isn't right, you should be using:
```
from airflow.sdk._shared.timezones import timezone
```
##########
airflow-core/tests/unit/cli/commands/test_dag_command.py:
##########
@@ -55,8 +54,14 @@
clear_db_runs,
parse_and_sync_to_db,
)
+from tests_common.test_utils.version_compat import AIRFLOW_V_3_1_PLUS
from unit.models import TEST_DAGS_FOLDER
+if AIRFLOW_V_3_1_PLUS:
+ from airflow.sdk.bases.operator import BaseOperator
+else:
+ from airflow.models.baseoperator import BaseOperator # type:
ignore[no-redef]
Review Comment:
I'd rather use try / except actually.
##########
airflow-core/tests/unit/cli/commands/test_dag_command.py:
##########
@@ -55,8 +54,14 @@
clear_db_runs,
parse_and_sync_to_db,
)
+from tests_common.test_utils.version_compat import AIRFLOW_V_3_1_PLUS
from unit.models import TEST_DAGS_FOLDER
+if AIRFLOW_V_3_1_PLUS:
+ from airflow.sdk.bases.operator import BaseOperator
+else:
+ from airflow.models.baseoperator import BaseOperator # type:
ignore[no-redef]
Review Comment:
Semantically, we could use v_3_0 here because the move for BaseOperator to
sdk was done in that timeframe (not that it will affect the functionality here)
##########
pyproject.toml:
##########
@@ -754,6 +754,7 @@ testing = ["dev", "providers.tests", "tests_common",
"tests", "system", "unit",
"kubernetes-tests/*" = ["D", "TID253", "S101", "TRY002"]
"helm-tests/*" = ["D", "TID253", "S101", "TRY002"]
"providers/**/tests/*" = ["D", "TID253", "S101", "TRY002"]
+"performance/tests/*" = ["S101"]
Review Comment:
Why this change? Did you encounter the flake8-bandit’s S101 rule?
--
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]
