potiuk commented on issue #52753: URL: https://github.com/apache/airflow/issues/52753#issuecomment-3095562314
# Approach 2: Update serializers list based on numpy version Yes that is the best solution I think. > However, the following failure raised in pre-commit checks. > Generate Datamodels for TaskSDK client > importlib.metadata.PackageNotFoundError: No package metadata was found for numpy > Would really appreciate if @bolkedebruin could have some time to look into it and share some insights. The reason why it's failing in CI is that our task-sdk (for a good reason) perform `uv sync` on task-sdk and only run tests with the dependencies of task-sdk and airflow-core - and "pandas" is (correctly) not one of the dependencies of neither of the two It's an optional dependency. And the failure clearly indicates that currently serialization has a strong dependncy on pandas being installed (which is wrong - serialization shoudl work without pandas, pandas serialization shoudl be an optional feature). This is exactly part of the problem that we have been discussing in the past with @bolkedebruin -> that depending in core on some of the external entities is a bad idea because then you need to add dependencies on things that you do not need. In this case TaskSDK depends on pandas implicitly, because serializer depends on it and this is pretty bad idea in general - especially when you have to have some ways of specifying that pandas dependency limits that potentially clash with other packages - in this case it's numpy and pandas together that do not work with each other in certain combinations. What really **should** be done, is that such pandas serializing capability should be optional because not every user of "serialization" will want to have pandas installed (especially that pandas, numpy and others are often conflicting with other pacakges and are **huge** dependencies in general). This could be eventually done in the way that I proposed and discussed with @bolkedebruin (but we could not get to a common conclusion) that serializer should be **truly** pluggable - in the way that we have done many features for providers - for example now message queue is **truly** pluggable - the "core" part implemented in "airflow-core" does not know which implementations are installed, it will dynamically discover and register them via provider's entrypoings (and many other airflow core features work like that) - this makes Airflow "extendable platform" where new implementations of non-core features are dynamically added. In this case pandas serialization is definitely not a "core" feature. Serialization is a core feature and it should be extended by adding "something" to extend it. This could be provider for example or we could use `pluggy` for that (we already use pluggy for listeners for example). For now the "intermediate" solution @sjyangkevin is to treat the "pandas serialization" in the core as optional even if it is implemented in the core: * catch the exception when you cannot read pandas package version * skip all the rest (for example return empty serialization) if the package cannot be found It's a bit "hacky" - the code for handling pandas would still be in "airflow-core" and we would pretend that pandas serialization does nothing, but it will work. But we are about to separate core features into "shared" code (see discussions in https://github.com/apache/airflow/pull/53149 and https://github.com/apache/airflow/pull/53417 and https://github.com/apache/airflow/pull/53506 and "Serialization" should be one of such "shared" functionalities that we will have to separate out, and probably while doing it we can think how to implement serialization in a truly pluggable way: cc: @ashb @kaxil @amoghrajesh - something to discuss how to do it. -- 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]
