henry3260 commented on code in PR #69627:
URL: https://github.com/apache/airflow/pull/69627#discussion_r3796572999
##########
airflow-ctl/src/airflowctl/api/operations.py:
##########
@@ -207,35 +207,44 @@ def __init_subclass__(cls, **kwargs):
if callable(value):
setattr(cls, attr,
_check_flag_and_exit_if_server_response_error(value))
- def execute_list(self, *, path, data_model, offset=0, limit=50,
params=None):
- if limit <= 0:
+ def execute_list(self, *, path, data_model, offset=0, limit=None,
params=None):
+ page_size = 50
+ if params:
+ limit = params.pop("limit", limit)
+ offset = params.pop("offset", offset)
+ if limit is not None and limit <= 0:
raise ValueError(f"limit must be a positive integer, got {limit}")
- shared_params = {"limit": limit, **(params or {})}
-
def safe_validate(content: bytes) -> BaseModel:
try:
return data_model.model_validate_json(content) # type:
ignore[union-attr]
except ValidationError:
raw = fill_missing_fields(json.loads(content), data_model)
return data_model.model_validate(raw) # type:
ignore[union-attr]
Review Comment:
> I'm not sure to get this. Similarly to what the API is doing, (limit is
the page size basically)
agree, IIUC we should implement like this
```
total_entries = first_pass.total_entries
stop = min(offset + limit, total_entries) if limit else total_entries
...
while offset < stop:
...
obj = data_model(**{found_key: entry_list, "total_entries": total_entries})
```
--
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]
