fix: pass order_by to Airflow API params in list adapters#230
Open
de-harish wants to merge 3 commits into
Open
fix: pass order_by to Airflow API params in list adapters#230de-harish wants to merge 3 commits into
de-harish wants to merge 3 commits into
Conversation
list_dag_runs, get_task_instances, list_asset_events, and list_import_errors were building params dicts without order_by, so the sort parameter was silently dropped even when callers passed it. Add order_by to each endpoint's params dict with sensible defaults so results are returned newest-first out of the box: - list_dag_runs / get_task_instances: -start_date - list_asset_events: -timestamp - list_import_errors: -import_error_id
/datasets/events and /assets/events do not support order_by — passing it causes a 400 Bad Request. Removed order_by from those API params; Airflow already returns asset events newest-first by default. /importErrors also does not support order_by. Switched to client-side sorting after fetching results so the default (-import_error_id) still gives most-recent-first behaviour without breaking the API call.
list_dags now accepts order_by, tags, dag_id_pattern, only_active and paused parameters so callers can filter and sort without post-processing. list_dag_runs now accepts state, start_date_gte and start_date_lte filters so callers can narrow to failed/running runs or a specific time window without fetching the full history. Abstract base updated to match both concrete adapters (v2, v3).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
list_dag_runs,get_task_instances,list_asset_events, andlist_import_errorsin both adapters (airflow_v2.pyandairflow_v3.py) were buildingparamsdicts without includingorder_by. The sort parameter was silently dropped even when explicitly passed by callers — for example,dag_run.pyalready exposedorder_by="-start_date"on the tool but the adapter discarded it before the HTTP call.Why
The Airflow REST API defaults to ascending order when no
order_byis provided, so all list tools were returning oldest-first regardless of what the caller requested.Changes
Added
order_byto each endpoint'sparamsdict with sensible defaults so results are returned newest-first out of the box:list_dag_runs-start_dateget_task_instances-start_datelist_asset_events-timestamplist_import_errors-import_error_idTesting
uvx ruff checkanduvx ruff format --check)test_adapters.pycontinue to pass (no assertions onorder_byparam needed changing)