karenbraganz commented on PR #39724: URL: https://github.com/apache/airflow/pull/39724#issuecomment-2278161976
@uranusjr @RNHTTR Here are different approaches in which i can implement this PR. I think the second approach is best in terms of maintaining a clear scope on PRs but let me know what you think. Since this PR is focused on making the pool_slots check available to partial() and _expand() ( to fix [issue #39639](https://github.com/apache/airflow/issues/39639), I could create the separate function and only include the pool_slots check in it for now. I will also open a separate PR to modify the other checks and add them to that function in the way we discussed. This would maintain the scope of this PR on fixing the issue. The downside to this approach is that I would be partially implementing the change of creating a separate function to unify operator checks between two different PRs. Alternatively, I could just add the pool_slots check to both partial() and _expand() functions only for this PR instead of using the separate function, and open a separate PR to create the function that unifies the repeated code in one place. This would maintain the scope of this PR and also have a separate PR with the scope of modifying the way we run operator checks (instead of having this scope scattered between two PRs). The last option would be to add all fields including pool_slots to the separate function. This would fix the issue and also change the way we run operator checks in a single PR, but I think this would make the scope of the PR too broad. Please let me know your thoughts and which approach you think is best. -- 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]
