-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
Deprecate AutoML Tables operators #39752
Deprecate AutoML Tables operators #39752
Conversation
f97c154
to
56d27e7
Compare
e8fe9df
to
919e157
Compare
919e157
to
0e2ddb8
Compare
There is no clear what we try to achieve here. Deprecate or Remove functional because there is not available in upstream service? There is quite a different between two of them If we deprecate something, than we keep it as is with just a warning until one of option happen
If there is not available anymore, we could raise an error, with clear reason why it removed. "This not working anymore due to sunset of service XXX please use YYY" |
@Taragolis I think we're trying to remove the operators, but I was led to believe that we can't just remove it without giving the users some time and hints about the alternatives. The exceptions that I placed in the __init__method of the operators should do just that - they raise an error informing the users that the operators can no longer be supported, and suggests an alternative, if it exists. |
Is this operators still work in current version of provider? |
@Taragolis No, because AutoML platform has deprecated the corresponding functionality several months ago. Please take a moment to read the description to the PR and my previous comments, I tried to explain the situation the best I could. |
When you told "AutoML platform has deprecated" you mean "AutoML removed / shutdown / some service no longer available / discontinued"? |
Yeah I've already I found information: https://cloud.google.com/vision/automl/docs/deprecations
We never deprecate this operators before, so we can't deprecate it now, as I mention before deprecate mean "you still could use by your risk but it remove in some time in the future" What we could do it is raise an error on class initialise (what you already done) or on attempt to import this operators by utilise PEP-562 # airflow/providers/google/cloud/operators/automl.py
...
def __getattr__(name: str):
# PEP-562: Lazy loaded attributes on python modules
if name == "AutoMLTablesUpdateDatasetOperator":
# Don't forget to remove class AutoMLTablesUpdateDatasetOperator, so it would be also gone from else where
raise ImportError("Here the message why we pending to remove this operator")
elif ...:
...
else:
raise AttributeError(f"module {__name__!r} has no attribute {name!r}") Now it is more about wording and timeframe when we could deprecate operators. We could deprecate them before March 31, 2024, it's too late now 😞 So we should change error message to something like that. It is necessary to mention that the reason for deletion is that the service is no longer available
And finally we should add information about removals in provider CHANGELOG.rst. airflow/airflow/providers/google/CHANGELOG.rst Lines 229 to 237 in 97e867f
airflow/airflow/providers/slack/CHANGELOG.rst Lines 37 to 52 in fe4605a
airflow/airflow/providers/microsoft/azure/CHANGELOG.rst Lines 458 to 463 in 97e867f
The same valid for the operators with already removed functional but it could be done by the separate PR |
If the Google service is not working anymore we can just remove the operators and it will not be considered a breaking change. We just need to explain this in with changelog entry |
e712859
to
3593c2c
Compare
3593c2c
to
982d092
Compare
@Taragolis I updated the error messages in accordance with the template that you proposed. Please check. @eladkal Do I need to add some info about the operators to CHANGELOG.rst or will it be added during the release? Update: the CI failed one check, it does not seem related to my PR |
You have to add it manually, during release process only auto generated will add automatically, just because release manager can't keep in the head all significant/breaking changes between the providers releases |
Updated CHANGELOG.rst |
8543ce6
to
8fb3ff5
Compare
8fb3ff5
to
9a13dc1
Compare
9a13dc1
to
a4eb18e
Compare
* Deprecate AutoML Table operators * Update providers.google.CHANGELOG.rst with info about AutoML shutdown
Deprecate AutoML Tables operators:
AutoMLTablesListTableSpecsOperator
AutoMLTablesListColumnSpecsOperator
AutoMLTablesUpdateDatasetOperator
and
AutoMLDeployModelOperator
.The Cloud AutoML platform has gone through several deprecations in the recent months. Some features were moved to Cloud Translation and Vertex AI, and some just stopped being supported.
The AutoML Tables operators are trying to make operations on AutoML datasets, such as getting specs of the tables contained within the datasets or updating the datasets. However after its deprecation, the only available functionality for AutoML is AutoML Translation, which is integrated into Cloud Translation and does not support tabular datasets. So we can query the datasets with the operators, but no tables or their columns will be found, making these operators useless and prone to errors.
Tabular datasets are available in Vertex AI, but unfortunately at this moment only
AutoMLTablesUpdateDatasetOperator
has a substitute Vertex AI operator with similar functions.AutoMLTablesListTableSpecsOperator
andAutoMLTablesListColumnSpecsOperator
have no substitutes.The functionality of
AutoMLDeployModelOperator
is also not supported by Cloud AutoML anymore. Instead theDeployModelOperator
from Vertex AI should be used.This PR is a continuation of #38673 .
^ 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.rst
or{issue_number}.significant.rst
, in newsfragments.