hussein-awala commented on code in PR #38942:
URL: https://github.com/apache/airflow/pull/38942#discussion_r1561837881
##########
airflow/cli/commands/task_command.py:
##########
@@ -97,7 +97,7 @@ def _get_dag_run(
dag: DAG,
create_if_necessary: CreateIfNecessary,
exec_date_or_run_id: str | None = None,
- session: Session | None = None,
+ session: Session = NEW_SESSION,
Review Comment:
Not needed here since we don't always use it (for example when
`create_if_necessary="memory" and exec_date_or_run_id is None`)
##########
tests/utils/test_sqlalchemy.py:
##########
@@ -134,7 +134,9 @@ def test_with_row_locks(
query = mock.Mock()
session = mock.Mock()
session.bind.dialect.name = dialect
+ session.get_bind().dialect.name = dialect
session.bind.dialect.supports_for_update_of = supports_for_update_of
+ session.get_bind().dialect.supports_for_update_of =
supports_for_update_of
Review Comment:
They are duplicated here, no?
##########
airflow/api_connexion/endpoints/xcom_endpoint.py:
##########
@@ -74,8 +74,8 @@ def get_xcom_entries(
# Match idx_xcom_task_instance + idx_xcom_key for performance.
query = query.order_by(XCom.dag_id, XCom.task_id, XCom.run_id,
XCom.map_index, XCom.key)
total_entries = get_query_count(query, session=session)
- query = session.scalars(query.offset(offset).limit(limit))
- return xcom_collection_schema.dump(XComCollection(xcom_entries=query,
total_entries=total_entries))
+ xcom_entries = session.scalars(query.offset(offset).limit(limit)).all()
+ return
xcom_collection_schema.dump(XComCollection(xcom_entries=xcom_entries,
total_entries=total_entries))
Review Comment:
Try to avoid updating the passed object type in this kind of PRs, for
example, the usage of `.all()` here could impact the performance if the dump
method handles the generator correctly, however, you can change the type hint
in `XComCollection` by adding generator as accepted type.
--
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]
