onlyarnav commented on PR #68543: URL: https://github.com/apache/airflow/pull/68543#issuecomment-4728698497
# 1 Sorry for the public exposure, that wasn't intentional, will prefix all four classes with an underscore so they are private. # 2.1 You're right that the the post-submit-commands handling in poll_until_complete ; finally block wasn't really backend specific, it was operator cleanup that happened to be duplicated across YARN and standalone. i'll pull it into a shared helper on the base class: each backend will just passes its tracking call in as a closure # 2.2 ### Thin wrapper observation by Claude Opus 4.8: Methods like `get_job_status` being a near-direct call to `hook.query_yarn_application_status()` is by design rather than an oversight: each backend method exists specifically to isolate the one call that varies per deployment mode, so a thin method at that seam is the abstraction working correctly, not leftover bloat. If we collapsed those down further we'd just be re-introducing the branching this refactor was meant to remove. # 3 On the class vs private methods point: I would lean toward keeping the class based approach. Looking at the issue background, standalone or YARN or K8s tracking were added in separate PRs over time, which suggests this is an area that keeps growing. With the strategy classes, adding a future backend means writing one new class against a fixed interface with no risk to existing backends or their tests otherwise with grouped private methods we will be back to editing six dispatch points per addition (basically the original problem). Let me the make the new commits asap -- 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]
