ashb commented on code in PR #57718:
URL: https://github.com/apache/airflow/pull/57718#discussion_r2487421558
##########
airflow-ctl/src/airflowctl/api/client.py:
##########
@@ -214,6 +231,79 @@ def __init__(
**kwargs,
)
+ def _clean_empty_values(self, data: dict[str, Any]) -> dict[str, Any]:
+ """
+ Recursively remove keys with None or empty values from a dictionary if
they are not required in the datamodels.
+
+ Args:
+ data (dict): The input dictionary to clean.
+ Returns:
+ dict: The cleaned dictionary with empty values removed.
+ """
+ if not self.datamodel:
+ return data
+ # Check each datamodel and clean if field is nullable
+ cleaned_data = {}
+ for datamodel in self.datamodel:
+ # Create map from field name and required status
+ required_fields = []
+ for field_name, field_info in
datamodel.__pydantic_fields__.items():
Review Comment:
Normally I would have just looked at `datamodel.model_fields.items()` --
does that not work here?
##########
airflow-ctl/src/airflowctl/api/operations.py:
##########
@@ -115,7 +115,33 @@ def _exit_if_server_response_error(response: Any |
ServerResponseError):
raise response
return response
+ def _class_exists(module, class_name):
+ """Check if a class with a given name exists in the imported module."""
+ import inspect
+
+ for name, obj in inspect.getmembers(module, inspect.isclass):
+ # Make sure the class is defined in this module, not imported
+ if obj.__module__ == module.__name__ and name == class_name:
+ return True
+ return False
Review Comment:
Using 'inspect' for this is quite heavy.
Woulding `return hasattr(module, class_name)` be 99% equivalent?
##########
airflow-ctl/src/airflowctl/api/client.py:
##########
@@ -214,6 +231,79 @@ def __init__(
**kwargs,
)
+ def _clean_empty_values(self, data: dict[str, Any]) -> dict[str, Any]:
+ """
+ Recursively remove keys with None or empty values from a dictionary if
they are not required in the datamodels.
+
+ Args:
+ data (dict): The input dictionary to clean.
+ Returns:
+ dict: The cleaned dictionary with empty values removed.
+ """
+ if not self.datamodel:
+ return data
+ # Check each datamodel and clean if field is nullable
+ cleaned_data = {}
+ for datamodel in self.datamodel:
+ # Create map from field name and required status
+ required_fields = []
+ for field_name, field_info in
datamodel.__pydantic_fields__.items():
+ if field_info.is_required():
+ required_fields.append(field_name)
+
+ for key, value in data.items():
+ if key in required_fields:
+ cleaned_data[key] = value
+ else:
+ # clean only if value is not None or empty
+ if value is not None:
+ if isinstance(value, dict):
+ cleaned_data[key] = self._clean_empty_values(value)
+ else:
+ cleaned_data[key] = value
+ return cleaned_data
+
+ # Override request method to include more validation
+ def request(
+ self,
+ method: str,
+ url: URL | str,
+ *,
+ content: RequestContent | None = None,
+ data: RequestData | None = None,
+ files: RequestFiles | None = None,
+ json: typing.Any | None = None,
+ params: QueryParamTypes | None = None,
+ headers: HeaderTypes | None = None,
+ cookies: CookieTypes | None = None,
+ auth: AuthTypes | UseClientDefault | None = USE_CLIENT_DEFAULT,
+ follow_redirects: bool | UseClientDefault = USE_CLIENT_DEFAULT,
+ timeout: TimeoutTypes | UseClientDefault = USE_CLIENT_DEFAULT,
+ extensions: RequestExtensions | None = None,
+ ) -> Response:
+ """Make a request with the client."""
+ # Include any validation below if needed
+ if json is not None:
+ # Validate JSON body before sending the request and cleanse empty
values
+ # This is needed because of model_dump including None values for
optional fields which send to API as empty
+ json = self._clean_empty_values(data=json)
Review Comment:
First thought: This feels out-of-place in the client class, which should
just be dealing with the HTTP layer.
Validation and processing should happen before the request is passed to the
client.
For example -- is it possible to do this processing via a pydantic base
model instead?
--
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]
