Add CloudSQLNoOperationInProgressSensor for parallel admin ops - #68151
Conversation
|
@Vamsi-klu A few things need addressing before review — see our Pull Request quality criteria.
No rush. Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this two-stage triage process so that our maintainers' limited time is spent where it matters most: the conversation with you. |
|
Progress update (2026-06-14): this PR is intentionally kept as the best-effort sensor lane for #68040 (it reduces collision likelihood only). It does not attempt the submit-time correctness fix for The active hook-level correctness fix is being tracked in #68361, which is still open and currently marked Environment note: in this environment, full unit/E2E execution is limited (no Docker runtime and no usable GCP config here), so full in-app-browser E2E could not be completed. |
c2c9e1e to
de93e52
Compare
de93e52 to
6ca5d91
Compare
|
Rebased this PR onto current I also tightened the new no-op sensor/trigger tests after the rebase: autospecced the new sensor hook patches and replaced the new trigger wait-path test's Scope note: this remains the best-effort, pre-submit orchestration path for Cloud SQL admin operations. It complements the hook-level retry work in #68361: hook retry handles a submit-time 409, while this deferrable sensor lets a Dag wait before submit without holding a worker slot. Validation run:
PR is now mergeable; required review/checks remain. Drafted-by: Codex (GPT-5) |
The sensor must be referenced by an example DAG to satisfy the Google provider project-structure test (test_missing_examples). It gates the import task on the instance having no admin operation in progress, which is the documented usage pattern.
- Raise a dedicated CloudSQLOperationError (a subclass of AirflowException) instead of the base AirflowException, so the new sensor passes the check-no-new-airflow-exceptions static check. - Add the new cloud_sql sensor to the generated get_provider_info.py. - Restore the correct generated/provider_dependencies.json.sha256sum (the .json itself is unchanged from main).
6ca5d91 to
e9d7e19
Compare
potiuk
left a comment
There was a problem hiding this comment.
Thanks — approving this as the surviving implementation of CloudSQLNoOperationInProgressSensor.
Context worth recording: #68146 proposes the same sensor and trigger, opened a day before this one. Two contributors built the same primitive in parallel and both sat for ~8 weeks, which is the worst outcome for everyone. I'm going with this one and closing that.
What decided it:
- This is mergeable; #68146 is conflicting and ~1042 commits behind.
CLOUD_SQL_NON_TERMINAL_STATUSESis factored into the hook and shared, rather than the status list being duplicated across sensor and trigger.CloudSQLOperationError(AirflowException)is a dedicated exception subclass — exactly what the project asks for instead of raising the broadAirflowExceptiondirectly. There are no new bareraise AirflowException(...)statements here.- Trigger tests live in
test_cloud_sql.py, mirroringtriggers/cloud_sql.py, per the test-location convention.
The sensor being operation-agnostic (waiting on any non-terminal admin operation rather than a specific one) is the right shape — it's the 409 operationInProgress case that Dag authors actually hit, and it composes with import/export/clone/patch without needing a variant per operation type.
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
What
Adds an operation-agnostic, deferrable
CloudSQLNoOperationInProgressSensorto the Google provider. The sensor waits until a Cloud SQL instance has no administrative operation in flight before downstream tasks submit a new one.Supporting changes:
CloudSQLHook.list_operations()forsqladmin.operations.list, filtered to the requested instance viatargetId.CLOUD_SQL_NON_TERMINAL_STATUSES(PENDING/RUNNING) to decide whether an operation is still in progress.CloudSQLNoOperationInProgressTriggerfor deferrable mode.provider.yaml, generated provider info, the Cloud SQL how-to docs, and the system-test example.Why
Cloud SQL serializes administrative operations per instance. Two parallel
CloudSQLImportInstanceOperator/CloudSQLExportInstanceOperatortasks against the same instance can race, and the second submit can fail immediately with HTTP 409operationInProgress.This sensor reduces that collision window by giving Dags an explicit pre-flight wait before an admin operation is submitted.
Relationship to #68361
This PR is complementary to the hook-level retry work in #68361, not a replacement for it.
The sensor remains best-effort. It cannot eliminate the race between sensor success and the next operator submit, but it gives users a reusable way to reduce avoidable collisions and to serialize mixed Cloud SQL admin operations before they occupy worker execution.
Testing
New/updated unit tests:
providers/google/tests/unit/google/cloud/hooks/test_cloud_sql.py::TestGcpSqlHookDefaultProjectId::test_list_operationsproviders/google/tests/unit/google/cloud/sensors/test_cloud_sql.pyproviders/google/tests/unit/google/cloud/triggers/test_cloud_sql.py::TestCloudSQLNoOperationInProgressTriggerValidation run after rebasing onto current
upstream/main:uv run ruff format providers/google/src/airflow/providers/google/cloud/hooks/cloud_sql.py providers/google/src/airflow/providers/google/cloud/sensors/cloud_sql.py providers/google/src/airflow/providers/google/cloud/triggers/cloud_sql.py providers/google/src/airflow/providers/google/get_provider_info.py providers/google/tests/system/google/cloud/cloud_sql/example_cloud_sql.py providers/google/tests/unit/google/cloud/hooks/test_cloud_sql.py providers/google/tests/unit/google/cloud/sensors/test_cloud_sql.py providers/google/tests/unit/google/cloud/triggers/test_cloud_sql.pyuv run ruff check --fix providers/google/src/airflow/providers/google/cloud/hooks/cloud_sql.py providers/google/src/airflow/providers/google/cloud/sensors/cloud_sql.py providers/google/src/airflow/providers/google/cloud/triggers/cloud_sql.py providers/google/src/airflow/providers/google/get_provider_info.py providers/google/tests/system/google/cloud/cloud_sql/example_cloud_sql.py providers/google/tests/unit/google/cloud/hooks/test_cloud_sql.py providers/google/tests/unit/google/cloud/sensors/test_cloud_sql.py providers/google/tests/unit/google/cloud/triggers/test_cloud_sql.pyuv run --project providers/google pytest providers/google/tests/unit/google/cloud/hooks/test_cloud_sql.py::TestGcpSqlHookDefaultProjectId::test_list_operations providers/google/tests/unit/google/cloud/sensors/test_cloud_sql.py providers/google/tests/unit/google/cloud/triggers/test_cloud_sql.py::TestCloudSQLNoOperationInProgressTrigger -xvsprek run ruff --from-ref upstream/mainprek run ruff-format --from-ref upstream/mainLive GCP E2E was not run in this environment.
Related to: #68040
Was generative AI tooling used to co-author this PR?
Generated-by: Codex (GPT-5); Claude Code (GPT-5) following the guidelines