potiuk commented on PR #38111:
URL: https://github.com/apache/airflow/pull/38111#issuecomment-2050167395
> But if you confirm this is fine then no problem I will do it like this :)
Yes. We do it in a number of other places - you already looked at it.
> couldn't we adapt the verify_providers module as that is the one that
causing the import issue and check that if the imported module isn't an ignored
providers from BASE_PROVIDERS_COMPATIBILITY_CHECKS?
Probably in this particular case it would work - but there are other
imports that are ignored for similar reasons - not only for "transfer"
operators. Example from aws security manager:
```
try:
from airflow.www.security_manager import AirflowSecurityManagerV2
except ImportError:
raise AirflowOptionalProviderFeatureException(
"Failed to import AirflowSecurityManagerV2. This feature is only
available in Airflow versions >= 2.8.0"
)
```
So the "optional feaature" is used for more things and here we can use it
easily.
Adding logic to handle import for excluded provider might add even more
complexity and needs someone to do it :) - but yes if you would like to do it
now for example that woudl work, you'd just need to pass the excluded providers
to the inside of the container image where the providers built are installed
in. The providers are imported inside the image that has all 690+ dependencies
installed, because otherwise a lot of them fail, so the import is done inside
Breeze image, and it adds a bit of complexity. Adding three imports sounds
easier (and likely can take a lot less time). But if you would like to do it -
go for it, I just feel you already spent a lot of time on this one :)
Some more comments:
* using local imports is one thing that we want to avoid - because this is
where compatibility checks will stop detecting errors altogether. and in your
example you could even simply not wrap it in optionalfeature exception. In this
case we **know** where the error comes from so we can add it.
> with optional_provider_feature():
Surely, but we have the same problem with common code. If we add it today,
it's only going to be really usable 10 months from now. because providers are
importing stuff from earlier airflow versions that won't have 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]
