potiuk commented on PR #46638:
URL: https://github.com/apache/airflow/pull/46638#issuecomment-2652302784

   I believe the approach I took in 
https://github.com/apache/airflow/pull/46608. that cleans-up a lot of common 
and duplicated provider code after we moved all providers is better.
   
   It should fix the deprecations_ignore.yml problem. After removing the "old" 
way providers were added and basically removing the entire "providers" package 
we could do the deprecations_ignore differently -  namely instead of adding 
deprecations_ignore purely for the provider in question, we are adding all 
deprecations_ignore found in all providers, ALWAYS in pytest_plugin. 
   
   This way:
   
   a) we can keep deprecations_ignore in corresponding providers
   b) no matter which provider is run, the ignores are added for all of them
   c) we do not have to copy around the code that reads the deprecations_ignore
   
   I have to still make the PR green - there are still few things failing 
there, and also add some more docs and templating/pre-commits to make sure that 
"integration" and "system" tests in provider have the right __init__.py files 
and few more things to remove, but this evening the tests in CI already look 
very promising -  I have most integration tests running and the deprecation 
warnings from google provider seem to be handled well - via the above mechanism.
   
   cc: @ashb 
   
   > (Also wow that's quite a lot of warnings to ignore! What warning are we 
actually ignoring here?)
   
   We are ignoring all the warnings that are deprecations that we **know** 
about and will remove in the future. Generally our approach is that our tests 
should be "warning free" and that's our end-goal, but in some cases we will 
have some intentional deprecations that we want to keep together with tests. 
This is the reason why we have this mechanism here to intentionally mark some 
of those deprecations as "expected"
   
   Possibly we can figure out even better way of doing it - such as marking 
certain deprecations as "ignorable" by the tests - which I think is a good idea 
and since we already have the specific deprecation warnings in Google, we might 
implement it and make similar approach for all other providers. But that should 
be one of the next steps I think after we clean-up stuff after provider move.
   
   Currently the problem is nearly not present, because as part of cleanup we 
removed all deprecation warnings (except the Google ones which we agred have 
already specific deprecation schedule and we are fine with it as discussed and 
agreed on the devlist), but we will have more of those "expected but ignorable" 
deprecation warnings as we introduce changes in providers - and maybe it's time 
to think about such mechanism
   
   


-- 
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]

Reply via email to