potiuk commented on PR #68146:
URL: https://github.com/apache/airflow/pull/68146#issuecomment-5147177765

   Thank you for this, and I'm sorry it's taken eight weeks to get you an 
answer.
   
   #68151 proposes the same feature — same 
`CloudSQLNoOperationInProgressSensor`, same 
`CloudSQLNoOperationInProgressTrigger`, near-identical file set — and was 
opened a day after yours. Two people built the same primitive in parallel and 
neither got reviewed, which is a failure of our process rather than of either 
contribution. I've merged #68151 and am closing this one.
   
   To be straight about why, since "the other one won" is not a useful answer:
   
   - **Mergeability.** #68151 is currently mergeable; this branch is ~1042 
commits behind `main` and conflicting, so landing it would have meant a 
substantial rebase of a 590-line feature.
   - **Shared status constant.** #68151 factors 
`CLOUD_SQL_NON_TERMINAL_STATUSES` into the hook so the sensor and trigger share 
one definition.
   - **Test placement.** Its trigger tests extend 
`tests/.../triggers/test_cloud_sql.py`, mirroring `triggers/cloud_sql.py`; this 
PR adds `test_cloud_sql_no_operation.py`, which doesn't follow the 
test-location convention.
   
   Your version was ahead on one point I want on record: it introduces **no** 
`AirflowException` usage at all. #68151 defines a 
`CloudSQLOperationError(AirflowException)` subclass — acceptable under the 
project's guidance, but yours avoided the base class entirely.
   
   One thing to check on your side for future PRs: this branch modifies 
`generated/provider_dependencies.json.sha256sum`, a generated checksum that 
shouldn't normally be hand-edited in a feature PR — worth confirming that 
wasn't an accidental inclusion.
   
   None of this reflects on the quality of your work. If you'd like to stay 
involved, a review on #68151 would be genuinely welcome — you know this problem 
space as well as anyone — and checking `gh pr list --search "<feature>"` before 
starting will stop this happening again.
   
   ---
   Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting


-- 
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]

Reply via email to