bugraoz93 commented on code in PR #50132:
URL: https://github.com/apache/airflow/pull/50132#discussion_r2217451271
##########
airflow-ctl/src/airflowctl/api/operations.py:
##########
@@ -147,6 +149,41 @@ 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: str,
+ data_model,
+ offset: int = 0,
+ limit: int = 50,
+ params: dict | None = None,
+ **kwargs,
+ ) -> type[BaseModel] | ServerResponseError:
+ shared_params = {**(params or {}), **kwargs}
+ try:
+ self.response = self.client.get(path, params=shared_params)
+ first_pass = data_model.model_validate_json(self.response.content)
+ for key, value in first_pass.model_dump().items():
+ if key != "total_entries" and isinstance(value, list):
+ data_entity = key
+ print(f"data_entity {data_entity}")
+ break
+ entry_list = getattr(first_pass,key)
Review Comment:
```suggestion
entry_list = getattr(first_pass, key)
```
##########
airflow-ctl/src/airflowctl/api/operations.py:
##########
@@ -592,13 +595,14 @@ def get(self, pool_name: str) -> PoolResponse |
ServerResponseError:
except ServerResponseError as e:
raise e
- def list(self) -> PoolCollectionResponse | ServerResponseError:
+ def list(self) -> list | ServerResponseError:
"""List all pools."""
- try:
+ """ try:
Review Comment:
Commented code
##########
airflow-ctl/src/airflowctl/api/operations.py:
##########
@@ -186,21 +223,18 @@ def get_by_alias(self, alias: str) -> AssetAliasResponse
| ServerResponseError:
except ServerResponseError as e:
raise e
- def list(self) -> AssetCollectionResponse | ServerResponseError:
+ def list(self) -> list | ServerResponseError:
"""List all assets from the API server."""
- try:
- self.response = self.client.get("assets")
- return
AssetCollectionResponse.model_validate_json(self.response.content)
- except ServerResponseError as e:
- raise e
+ return super().execute_list(path="assets",
data_model=AssetCollectionResponse)
def list_by_alias(self) -> AssetAliasCollectionResponse |
ServerResponseError:
"""List all assets by alias from the API server."""
- try:
+ """ try:
Review Comment:
Let's delete these commented-out parts :)
##########
airflow-ctl/src/airflowctl/api/operations.py:
##########
@@ -147,6 +149,41 @@ 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: str,
+ data_model,
+ offset: int = 0,
+ limit: int = 50,
+ params: dict | None = None,
+ **kwargs,
+ ) -> type[BaseModel] | ServerResponseError:
+ shared_params = {**(params or {}), **kwargs}
+ try:
+ self.response = self.client.get(path, params=shared_params)
+ first_pass = data_model.model_validate_json(self.response.content)
+ for key, value in first_pass.model_dump().items():
+ if key != "total_entries" and isinstance(value, list):
+ data_entity = key
+ print(f"data_entity {data_entity}")
Review Comment:
```suggestion
```
##########
airflow-ctl/src/airflowctl/api/operations.py:
##########
@@ -362,13 +392,14 @@ def get(self, conn_id: str) -> ConnectionResponse |
ServerResponseError:
except ServerResponseError as e:
raise e
- def list(self) -> ConnectionCollectionResponse | ServerResponseError:
+ def list(self) -> list | ServerResponseError:
"""List all connections from the API server."""
- try:
+ """ try:
Review Comment:
Same here for the commented code
##########
airflow-ctl/src/airflowctl/api/operations.py:
##########
@@ -569,16 +578,10 @@ def create(
class JobsOperations(BaseOperations):
"""Job operations."""
- def list(
- self, job_type: str, hostname: str, is_alive: bool
- ) -> JobCollectionResponse | ServerResponseError:
+ def list(self, job_type: str, hostname: str, is_alive: bool) -> list |
ServerResponseError:
"""List all jobs."""
- try:
- params = {"job_type": job_type, "hostname": hostname, "is_alive":
is_alive}
- self.response = self.client.get("jobs", params=params) # type:
ignore
- return
JobCollectionResponse.model_validate_json(self.response.content)
- except ServerResponseError as e:
- raise e
+ params = {"job_type": job_type, "hostname": hostname, "is_alive":
is_alive}
+ return super().execute_list(path="jobs",
data_model=JobCollectionResponse, params=params,offset = 2,limit = 5)
Review Comment:
Do we need to send offset and limit? Maybe we can even delete this from the
method definition if we won't ask the user on the `page` and `limit` :) We can
do that enhancement as a next step, let's first not have the ability to set
them and we return all
--
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]
