pierrejeambrun commented on code in PR #42959:
URL: https://github.com/apache/airflow/pull/42959#discussion_r1804393186
##########
airflow/api_fastapi/parameters.py:
##########
@@ -129,6 +129,26 @@ def transform_aliases(self, value: str | None) -> str |
None:
return value
+class _OrderByParam(BaseParam[str]):
Review Comment:
I don't think we need that, this is really niche because most of the list
endpoints have multiple order possibility. And that's some code duplication,
and it is missing a few things (secondary ordering to enforce non flakyness of
returned order in case the first criteria is equal)
Then as a user you have to think:
- Is there multiple sort possiblities, then use `?order_by=xxx` put a field
with `-` and ` `
- There is not, then user `?order_by=xxx, with `asc` and `desc`
Mixing two concept in the same query_param.
I think we should remove it and use the systematic `order_by=field` or
`order_by=-field` and I should be ready for to merge. (or at least very close
to)
--
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]
