Remove deprecated "delegate_to" from GCP operators and hooks #30748
Conversation
boring-cyborg
bot
added
provider:cncf-kubernetes
shahar1
force-pushed
the
remove-delegate-to
branch
from
c2f547d to
8aac2f4
Compare
There was a problem hiding this comment.
Need also the following:
- Add to all relevant provider.yaml with new breaking change version (Except google as there it's already set)
- Add entry in changelog of each provider that explains the breaking change for the specific provider. This is simply an entry of "Removed delegate_to parameter from operators x, y, z"
shahar1
force-pushed
the
remove-delegate-to
branch
from
8aac2f4 to
091fa30
Compare
Done |
shahar1
force-pushed
the
remove-delegate-to
branch
from
7c484e7 to
ae5f970
Compare
| Breaking changes | ||
| ~~~~~~~~~~~~~~~~ | ||
|
|
||
| * ``Remove deprecated 'delegate_to' param from transfers that interact with Google Cloud (#30748)`` |
There was a problem hiding this comment.
This line will be added automatically by the release manager during release.
What we want to describe via the PR is information about that the commit message itself does not cover.
For example: what operators involve? what action users needs to take.. basically additional information that may help user to bridge the breaking changes faster
| @@ -152,16 +151,10 @@ class GCSHook(GoogleBaseHook): | |||
| def __init__( | |||
| self, | |||
| gcp_conn_id: str = "google_cloud_default", | |||
| delegate_to: str | None = None, | |||
There was a problem hiding this comment.
I think we cannot just remove the parameter here, because this will break an important property of the providers we have that they can be upgraded independently.
While I see why removing such deprecated parameter is needed at some point in time doing it this way has rather profound consequences.
For example if somene would like to keep old AWS provider but would like to upgrade a google provider to the new version, this will break silently for AWS provider (which will not be upgraded) - even if delegate_to parameter has NOT been used.
The problem is that old providers are already passing this parameter even if it has not been used. It will default to None and None will passed down.
So for exmple if you have old AWS provider and new Google provider, and you do not have delegate_to parameter in your operator, the gcs_to_s3 provider will call the GCSHook with "delegate_to" parameter set to None and this will crash.
This is far too invasive. Yes. It's ok to remove that feature, but it's not ok to break another provider's usage of this hook even if the feature has not been used.
Proposal:
I am ok with removing the delegate to parameter, but this change should be done differently.
-
In all Google hooks using
delegate_to, remove thedelegate_toparameter but add**kwargs. This will stop the Hooks from crashing even ifdelegate_to=Noneparameter has been passed to it from an older version of Amazon or other providers. -
In those hooks convert the warning into error. Smth like:
if kwargs.get('delegate_to') is not None:
raise RuntimeError("The `delegate_to` parameter has been deprecated before and finally removed in this version of Google Provider. You MUST convert it to `impersonate_chain`).
I think this should be left here pretty much indefinitely.
There was a problem hiding this comment.
Done, including tests for the if statements in all GCP hooks
shahar1
force-pushed
the
remove-delegate-to
branch
from
ae5f970 to
07e1a71
Compare
Two PRs crossed and the result of apache#30748 caused the apache#30579 to fail as delegate_to parameter has been removed.
|
Hi! |
Restore delegate_to param to GoogleDiscoveryApiHook to allow to specification of a service account for domain-wide delegation when using GoogleDiscoveryApiHook to access Google APIs which support domain-wide delegation but do not have a dedicated hook (such as Workspaces Admin API). Note that based on the discussion in apache#9461, the delegate_to param was deprecated in apache#29088 (and removed in apache#30748) from many hooks including GoogleDiscoveryApiHook. However, the delegate_to param was not removed from the docstring for the GoogleDiscoveryApiHook constructor. Update GCP connection docs to reflect delegate_to param in GoogleDiscoveryApiHook usage only when using Google APIs that support domain-wide delegation.
Restore delegate_to param to GoogleDiscoveryApiHook to allow to specification of a service account for domain-wide delegation when using GoogleDiscoveryApiHook to access Google APIs which support domain-wide delegation but do not have a dedicated hook (such as Workspaces Admin API). Note that based on the discussion in #9461, the delegate_to param was deprecated in #29088 (and removed in #30748) from many hooks including GoogleDiscoveryApiHook. However, the delegate_to param was not removed from the docstring for the GoogleDiscoveryApiHook constructor. Update GCP connection docs to reflect delegate_to param in GoogleDiscoveryApiHook usage only when using Google APIs that support domain-wide delegation.
related: #9461, #29088
delegate_toparameter from GCP operators, hooks, sensors, transfers, and triggers, as well as from firebase hook.delegate_toparam will still be available only in gsuite and marketing platform hooks, operators, sensors, and transfers that don't interact with Google Cloud (see reasoning in Unclear documentation for the delegate_to parameter #9461).^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an 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 a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in newsfragments.