potiuk commented on PR #32319:
URL: https://github.com/apache/airflow/pull/32319#issuecomment-1621060022
> I'm suggesting that because I like the solution of adding a serialization
method that other hooks can override. Like this, the right tool is added at the
right place. And can be used for both ODBC and Databricks. Otherwise, doing
this change without touching the common.sql hook feels like hacking and working
around the run method (and it add a new flavor of this method in the codebase).
If we do it like this, then we should add "additional-dependencies" in
provider.yaml and add common.sql >= NEXT_COMMON_SQL_VERSION and bump the
common.sql version to next MINOr version in their provider.yaml.
This would be effectively a new feature of common.sql that those two
providers would depend on so making a new MINOR (i.e. feature) release of
common.sql and make those two provider depends on it is the only way to do it.
It's possible but a little dangerous because all other providers that depend on
common.sql could start relying on this feature (i.e. add their own
make_serializable) implementation in the futre, without adding the >= - so the
only "future-proof" way of adding this would also be to add a pre-commit that
will enforce that if a provider uses `make_serializable`, it also has
common.sql >= NEW_COMMON_SQL_VERSION, or - alternatively - we add common.sql >=
NEW_COMMON_SQL_VERSION in all providers that use it and make it a new
expectation for all providers, regardless if they are using the new feature
(this also seems to be workable solution and will likely also require a
pre-commit to check it in relevant provider.yaml files).
So yeah. It's possible to do it the way you propose - but will require a bit
of build/CI overhead to make it robust and prevent accidental mistakes in the
future.
--
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]
