bugraoz93 commented on PR #50175: URL: https://github.com/apache/airflow/pull/50175#issuecomment-2887563668
> > A more general thing about routes and services: There are similar cases where we have methods inside route files. We already have some level of distinction between them. How about moving them to services? What do you think? cc: @pierrejeambrun @rawwar @jason810496 @shubhamraj-git @ephraimbuddy > > Looks good to me to move the query helper function to the `airflow-core/src/airflow/api_fastapi/core_api/services` module level. (I think `airflow-core/src/airflow/api_fastapi/common/db` would also be a suitable location.) > > Just to double-check, do you mean to move the `_find_task_instance_for_try_number` function directly into the `services` module and not introduce a new service class, right? > > Since this helper simply wraps a query as a function, and the existing service classes (e.g., for connections, pools, variables) are designed for bulk operations, creating a new class might not be necessary here. Yes, I agree on the second look, `airflow-core/src/airflow/api_fastapi/common/db` is more suitable. I intend to decouple these methods from routes. Even with `collapse all` in some routes, there are a lot of endpoints and making the code less readable. Yes, it can be on the root level. For sure, it depends on the use case if we look at it in general. We aren't doing full object-oriented programming in general, it is mixed with Functional mostly due to the nature of Python. Unless we use service classes as a singleton across all routes, it can only add overhead of creating the object rather than calling the method. I don't have a strong opinion on that since the entire stack is using a mixed approach and makes sense in most cases :) -- 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: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
