AlejandroMorgante commented on PR #69930: URL: https://github.com/apache/airflow/pull/69930#issuecomment-4983703572
> I am not entirely sold on this PR. > > Airflow is typically quite conservative about deprecating public operators, as they're imported directly into users' DAGs and therefore represent one of the most user-facing parts of the API. In my experience reviewing provider PRs, most deprecations have been for parameters, methods or other API elements, rather than replacing an entire operator. > > I agree that dedicated create/update/run/delete operators make each task's intent clearer. However, I'm not yet convinced that this alone justifies introducing a deprecation cycle. The existing GlueCrawlerOperator already provides this functionality and has been part of the provider API for some time. > > Could you elaborate on why deprecating the existing operator is the preferred approach here? Is there a limitation or design issue with the current operator that cannot reasonably be addressed while preserving the existing public API? Given the migration cost for existing users and the increase in public API surface, I think the rationale for deprecating an operator should be fairly strong. > > Also. have you asked @o-nikolas and/or @vincbeck before opening this PR? If you are proposing a deprecation cycle (I know it is 'just an operator' but Glue is a popular service), it would be better to ensure the maintainers are aligned so that you dont waste time on a PR that would just get closed. Thanks, this is a fair concern. The limitation I am trying to address is that `GlueCrawlerOperator` does not merely expose several optional operations. Its `execute()` method **always reconciles the crawler before starting it**: 1. It checks whether the crawler exists. 2. It creates it when missing or updates it when present. 3. It starts the crawler. Therefore, a Dag whose only responsibility is to run an existing crawler cannot express: > Run this crawler without modifying infrastructure. The task also requires create/update permissions, may recreate a crawler that was intentionally removed, and may apply configuration changes during what appears to be a routine execution task. That is the main motivation for the dedicated operators: they allow **infrastructure provisioning, configuration changes, execution, and deletion** to have separate ownership and narrower IAM permissions. The benefit is not only clearer naming; it is also preventing a run-only task from mutating infrastructure. That said, I agree that this does not necessarily require deprecating `GlueCrawlerOperator`. There is no compatibility limitation preventing us from keeping it as a supported composite operator while adding the operation-specific alternatives. My initial intention with the deprecation was to guide new Dags toward the narrower operators, not because preserving the existing API was impossible. Given the migration cost and Airflow's conservative approach to public operator deprecations, **I would be happy to remove the deprecation and make the proposal entirely additive**. Existing users could continue using `GlueCrawlerOperator`, while users who need isolated responsibilities and narrower permissions could use the new operators. I had already reached out to Vincent about this, and I am currently waiting for his review. Would keeping `GlueCrawlerOperator` fully supported while adding the dedicated operators address your concern? -- 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]
