bugraoz93 commented on PR #44332: URL: https://github.com/apache/airflow/pull/44332#issuecomment-2557854580
>Can you check that limit is actually being respected? In my manual testing, I was trying to only fetch 14 runs but I always got the default of 100. @pierrejeambrun would you have any ideas here? I have checked and it seems like it is working fine. I have included a test case for it. >Can we add a test case for a triply nested dag group? Because it looks like we're only bubbling a failed state up twice. Indeed, this needs to be recalculated since the entire parent state is updated accordingly. I have included that small calculation to cascade this to all recursive parent groups. I have also included test cases for nested loops. I have tested locally with a case that is exactly similar to the picture you sent with 3 depth. >As long as we have tests covering those edge cases we should be fine. I have included multiple test cases. I have also updated the code according to the comments. I will resolve them accordingly. Only one specific thing to mention in general is I have included the `SortParam` `OR method`. Since the `value` won't be `null`, the only thing to compare is the generated way which falls to the primary key. This made me eliminate the unnecessary method and make or with `order_by | SortParam(...., <first_element_ordering>)`. If you think this isn't generic enough to put on SortParam, I can move the OR logic to the grid method. --- Sorry for the delay! Many thanks for the tests and reviews! -- 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]
