vandonr-amz commented on PR #30699:
URL: https://github.com/apache/airflow/pull/30699#issuecomment-1513458550

   Nice small change with a big impact :)
   I'm not familiar with SQLAlchemy, so I'll rephrase my comprehension of your 
change here to make sure I understand well what you're doing (and tell me if 
I'm wrong):
   The method `next_dagruns_to_examine` claimed to return a List here
   
https://github.com/aws-mwaa/upstream-to-airflow/blob/1ebeb19bf7542850fff2f1e2f9795ad70c1b24e2/airflow/models/dagrun.py#L294
   but this type annotation was wrong, as it was returning a query, which was 
lazily ran every time it was iterated.
   So calling the method once but iterating twice on the results resulted in 
the query being executed twice.
   
   Adding this `.all()` forces evaluation on the spot and actually returns a 
List.
   
   Did I get this right ?
   
   Did you measure the improvement brought by this PR ? If so, how ? Do you 
have any result to share ?


-- 
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]

Reply via email to