Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove all deprecation warnings in providers #17900

Merged
merged 1 commit into from
Aug 30, 2021

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Aug 29, 2021

There were a number of deprecation warnings when just importing
certain providers or their examples. Some of them came from still
using 1.10 classes, some from using still apply_defaults and
some from badly implemented fallback mechanism to support
backwards compatibility.

This has all been fixed and our documentation generation step
for providers in CI will also fail in case there are some new
DeprecationWarnings generated, which means that they will have
to be fixed before merging.

While checking that we found that BigQuery table_resource new
approach had some inconsistencies when passing new table_resource
parameters. Some deprecated parameters should be None by default,
otherwise you'd have to explicitly set them to None when
table_resource is set but when it is not set, the backwards
compatible behaviour is maintained - the parameters get the same.
default values set.

Several deprecation warnings had wrong stack-level - this has been
fixed as well.

The "legitimate" warnings are now filtered out and they are only
filtered out for the appropriate modules we know they are generated
from or in case the warnings result from direct import of the
deprecated module.


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

@boring-cyborg boring-cyborg bot added area:dev-tools provider:cncf-kubernetes Kubernetes provider related issues area:providers provider:Apache provider:amazon-aws AWS/Amazon - related issues provider:google Google (including GCP) related issues labels Aug 29, 2021
@potiuk potiuk requested review from mik-laj and dimberman August 29, 2021 20:40
@potiuk potiuk force-pushed the cleanup-warnings-from-providers branch 2 times, most recently from a6f7f87 to 6e9435b Compare August 29, 2021 21:01
@potiuk potiuk force-pushed the cleanup-warnings-from-providers branch from 6e9435b to 5a9d79b Compare August 30, 2021 08:43
@potiuk potiuk requested a review from vikramkoka as a code owner August 30, 2021 08:43
@potiuk potiuk changed the title Remove all Deprecation warnings in providers Remove all deprecation warnings in providers Aug 30, 2021
@potiuk potiuk force-pushed the cleanup-warnings-from-providers branch from 5a9d79b to bd34146 Compare August 30, 2021 08:45
@potiuk potiuk requested a review from eladkal August 30, 2021 08:45
@potiuk
Copy link
Member Author

potiuk commented Aug 30, 2021

I'd love to merge that one before releasing the new wave of providers #17890, this way all examples and code will be "warning-free". We had many deprecation warnings generated either in examples or when simply importing the providers. This will prevent it from happening also in the future as is checked in the CI.

@uranusjr
Copy link
Member

I don’t understand the Big Query stuff; everything else LGTM.

Copy link
Contributor

@eladkal eladkal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks ok. also the BigQuery :)
It's just changing deprecated operators to the updated ones and align the parameters with updated version.

There are failure though in some tests so it needs to be fixed

@github-actions github-actions bot added the okay to merge It's ok to merge this PR as it does not require more tests label Aug 30, 2021
@github-actions
Copy link

The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.

@potiuk potiuk force-pushed the cleanup-warnings-from-providers branch from bd34146 to e830171 Compare August 30, 2021 09:33
@potiuk potiuk force-pushed the cleanup-warnings-from-providers branch from e830171 to e4561b8 Compare August 30, 2021 10:29
There were a number of deprecation warnings when just importing
certain providers or their examples. Some of them came from still
using 1.10 classes, some from using still `apply_defaults` and
some from badly implemented fallback mechanism to support
backwards compatibility.

This has all been fixed and our documentation generation step
for providers in CI will also fail in case there are some new
DeprecationWarnings generated, which means that they will have
to be fixed before merging.

While checking that we found that BigQuery table_resource new
approach had some inconsistencies when passing new `table_resource`
parameters. Some deprecated parameters should be None by default,
otherwise you'd have to explicitly set them to None when
`table_resource` is set but when it is not set, the backwards
compatible behaviour is maintained - the parameters get the same.
default values set.

Several deprecation warnings had wrong stack-level - this has been
fixed as well.

The "legitimate" warnings are now filtered out and they are only
filtered out for the appropriate modules we know they are generated
from or in case the warnings result from direct import of the
deprecated module.
@potiuk potiuk force-pushed the cleanup-warnings-from-providers branch from e4561b8 to 27b1991 Compare August 30, 2021 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:dev-tools area:providers okay to merge It's ok to merge this PR as it does not require more tests provider:amazon-aws AWS/Amazon - related issues provider:cncf-kubernetes Kubernetes provider related issues provider:google Google (including GCP) related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants