sjyangkevin commented on PR #51059: URL: https://github.com/apache/airflow/pull/51059#issuecomment-2986573314
Hi @bolkedebruin , I've pushed the refactor for all the interfaces to use `cls` instead of `classname`. Below are change highlights. 1. Update the interface of `deserialize` to `deserialize(cls: type, version: int, data: object)` 2. Group `_is_namedtuple` and `_is_pydantic_model` with other private methods 3. Use a constant `PYDANTIC_MODEL_QUALNAME` for "pydantic.main.BaseModel" 4. Remove import private method from `serde.py` in serializers 5. Add more unit test cases (bignum, builtin, pydantic) It would be great if I could get your guidance on how to better implement the following. Thank you for your time and patient in reviewing it. 1. In `deserialize`, I use `class: type` as the type hint, do you have any suggestion on this? 2. Use attributes to identify Pydantic model. Now, I have another private method in the pydantic serializer, which is duplicate code of the one in `serde.py`. Is there anywhere we can move it to a common place and safely import by both? I am thinking about `airflow.utils`. Or, we should change the way how it's checked. 3. I wasn't sure how to properly test iceberg, and deltalake, and there are a few pendulum (e.g., v2) tests are skipped. Is there any service or things I can setup locally to run those tests? 4. The test case `test_timezone_deserialize_zoneinfo` tries to deserialize "backports.zoneinfo.ZoneInfo". However, this module seems not in the breeze environment, and cannot be passed as `backports.zoneinfo.ZoneInfo`. I've read the PR link and discussion shared by @potiuk , I also think that is a good way to go, and I am interested in contributing to that part as the next step. Please feel free to let me know if anything needs to be changed, I would really appreciate any feedback and eager to make it better. I am having some issues running pre-commit locally (it's extremely slow). If the push didn't pass the checks, will make sure all checks pass before next push. -- 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]
