Copilot commented on code in PR #64933: URL: https://github.com/apache/airflow/pull/64933#discussion_r3066484873
########## providers/common/compat/src/airflow/providers/common/compat/security/permissions.py: ########## @@ -16,8 +16,16 @@ # under the License. from __future__ import annotations +from airflow.providers.common.compat.version_compat import AIRFLOW_V_3_0_PLUS + # Resource Constants RESOURCE_BACKFILL = "Backfills" RESOURCE_DAG_VERSION = "DAG Versions" -RESOURCE_ASSET = "Assets" RESOURCE_ASSET_ALIAS = "Asset Aliases" + +if AIRFLOW_V_3_0_PLUS: + RESOURCE_ASSET = "Assets" +else: + from airflow.security.permissions import ( + RESOURCE_DATASET as RESOURCE_ASSET, # noqa: F401 # type: ignore[attr-defined, no-redef] + ) Review Comment: The conditional import of `RESOURCE_DATASET` happens after module-level assignments (`RESOURCE_BACKFILL`, etc.), which will trigger Ruff E402 (imports not at top of file) for this module. To avoid CI/lint failures, move the `if AIRFLOW_V_3_0_PLUS` block (and the import) above the resource constant assignments, or otherwise ensure the import occurs before any non-import statements (alternatively, use a literal string for the AF2 value if you want to avoid conditional importing). ########## providers/common/compat/src/airflow/providers/common/compat/security/permissions.py: ########## @@ -16,8 +16,16 @@ # under the License. from __future__ import annotations +from airflow.providers.common.compat.version_compat import AIRFLOW_V_3_0_PLUS + # Resource Constants RESOURCE_BACKFILL = "Backfills" RESOURCE_DAG_VERSION = "DAG Versions" -RESOURCE_ASSET = "Assets" RESOURCE_ASSET_ALIAS = "Asset Aliases" + +if AIRFLOW_V_3_0_PLUS: + RESOURCE_ASSET = "Assets" +else: + from airflow.security.permissions import ( + RESOURCE_DATASET as RESOURCE_ASSET, # noqa: F401 # type: ignore[attr-defined, no-redef] + ) Review Comment: This change fixes a version-dependent permission constant, but there’s no regression test that asserts the *value* of `RESOURCE_ASSET` for the running Airflow major version (the existing import-only test won’t catch the Airflow 2.x vs 3.x mismatch). Add an assertion-based unit test that checks the expected value under Airflow 2.x vs 3.x (e.g., branch on `AIRFLOW_V_3_0_PLUS`). -- 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]
