You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "e-galan (via GitHub)" <gi...@apache.org> on 2024/05/22 08:32:17 UTC
[PR] Deprecate AutoMLTablesListTableSpecsOperator and AutoMLTablesListColumnSpecsOperator [airflow]
e-galan opened a new pull request, #39752:
URL: https://github.com/apache/airflow/pull/39752
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
Deprecate `AutoMLTablesListTableSpecsOperator` and `AutoMLTablesListColumnSpecsOperator`, update related tests.
Both `AutoMLTablesListTableSpecsOperator` and `AutoMLTablesListColumnSpecsOperator` are trying to make operations on AutoML datasets, such as getting specs of the tables contained withing the datasets, or of the tables' columns. However after its deprecation, the only available functionality in AutoML is AutoML Translation, which is integrated into Google 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 we do not have Vertex AI operators with similar functions.
This PR is a continuation of #38673 .
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
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](https://github.com/apache/airflow/tree/main/newsfragments).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate AutoML Tables operators [airflow]
Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis commented on PR #39752:
URL: https://github.com/apache/airflow/pull/39752#issuecomment-2127419940
When you told "AutoML platform has **deprecated**" you mean "AutoML **removed** / **shutdown** / **some service no longer available**"?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate AutoML Tables operators [airflow]
Posted by "e-galan (via GitHub)" <gi...@apache.org>.
e-galan commented on code in PR #39752:
URL: https://github.com/apache/airflow/pull/39752#discussion_r1611710250
##########
tests/providers/google/cloud/operators/test_automl.py:
##########
@@ -737,53 +725,45 @@ def test_templating(self, create_task_instance_of_operator):
class TestAutoMLTablesListTableSpecsOperator:
+ expected_deprecation_warning = (
+ "Class `AutoMLTablesListTableSpecsOperator` has been deprecated and will be removed after 31.12.2024. "
+ "AutoML platform no longer supports tabular datasets after its integration with Cloud Translation and Vertex AI. "
+ "Please refer to https://cloud.google.com/vertex-ai/docs/start/migrating-to-vertex-ai and "
+ "https://cloud.google.com/translate/docs/advanced/automl-upgrade for more info about available alternatives."
+ )
+
@mock.patch("airflow.providers.google.cloud.operators.automl.CloudAutoMLHook")
def test_execute(self, mock_hook):
filter_ = "filter"
page_size = 42
- op = AutoMLTablesListTableSpecsOperator(
- dataset_id=DATASET_ID,
- location=GCP_LOCATION,
- project_id=GCP_PROJECT_ID,
- filter_=filter_,
- page_size=page_size,
- task_id=TASK_ID,
- )
- op.execute(context=mock.MagicMock())
- mock_hook.return_value.list_table_specs.assert_called_once_with(
- dataset_id=DATASET_ID,
- filter_=filter_,
- location=GCP_LOCATION,
- metadata=(),
- page_size=page_size,
- project_id=GCP_PROJECT_ID,
- retry=DEFAULT,
- timeout=None,
- )
+ with pytest.raises(AirflowProviderDeprecationWarning, match=self.expected_deprecation_warning):
+ _ = AutoMLTablesListTableSpecsOperator(
+ dataset_id=DATASET_ID,
+ location=GCP_LOCATION,
+ project_id=GCP_PROJECT_ID,
+ filter_=filter_,
+ page_size=page_size,
+ task_id=TASK_ID,
+ )
+ mock_hook.assert_not_called()
@pytest.mark.db_test
def test_templating(self, create_task_instance_of_operator):
- ti = create_task_instance_of_operator(
- AutoMLTablesListTableSpecsOperator,
- # Templated fields
- dataset_id="{{ 'dataset-id' }}",
- filter_="{{ 'filter-' }}",
- location="{{ 'location' }}",
- project_id="{{ 'project-id' }}",
- impersonation_chain="{{ 'impersonation-chain' }}",
- # Other parameters
- dag_id="test_template_body_templating_dag",
- task_id="test_template_body_templating_task",
- execution_date=timezone.datetime(2024, 2, 1, tzinfo=timezone.utc),
- )
- ti.render_templates()
- task: AutoMLTablesListTableSpecsOperator = ti.task
- assert task.dataset_id == "dataset-id"
- assert task.filter_ == "filter-"
- assert task.location == "location"
- assert task.project_id == "project-id"
- assert task.impersonation_chain == "impersonation-chain"
Review Comment:
Because `AirflowException` is raised in the operator's `__init__` method. Therefore we cannot instantiate it and run any methods.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate AutoML Tables operators [airflow]
Posted by "e-galan (via GitHub)" <gi...@apache.org>.
e-galan commented on PR #39752:
URL: https://github.com/apache/airflow/pull/39752#issuecomment-2127330910
> 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.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate AutoMLTablesListTableSpecsOperator and AutoMLTablesListColumnSpecsOperator [airflow]
Posted by "e-galan (via GitHub)" <gi...@apache.org>.
e-galan commented on code in PR #39752:
URL: https://github.com/apache/airflow/pull/39752#discussion_r1610768094
##########
tests/providers/google/cloud/operators/test_automl.py:
##########
@@ -379,63 +379,51 @@ def test_templating(self, create_task_instance_of_operator):
assert task.impersonation_chain == "impersonation-chain"
-class TestAutoMLListColumnsSpecsOperator:
+class TestAutoMLTablesListColumnsSpecsOperator:
+ expected_deprecation_warning = (
+ "Class `AutoMLTablesListColumnSpecsOperator` has been deprecated and will be removed after 31.12.2024. "
+ "AutoML platform no longer supports tabular datasets after its integration with Cloud Translation and Vertex AI. "
+ "Please refer to https://cloud.google.com/vertex-ai/docs/start/migrating-to-vertex-ai and "
+ "https://cloud.google.com/translate/docs/advanced/automl-upgrade for more info about available alternatives."
+ )
+
@mock.patch("airflow.providers.google.cloud.operators.automl.CloudAutoMLHook")
def test_execute(self, mock_hook):
table_spec = "table_spec_id"
filter_ = "filter"
page_size = 42
- op = AutoMLTablesListColumnSpecsOperator(
- dataset_id=DATASET_ID,
- table_spec_id=table_spec,
- location=GCP_LOCATION,
- project_id=GCP_PROJECT_ID,
- field_mask=MASK,
- filter_=filter_,
- page_size=page_size,
- task_id=TASK_ID,
- )
- op.execute(context=mock.MagicMock())
- mock_hook.return_value.list_column_specs.assert_called_once_with(
- dataset_id=DATASET_ID,
- field_mask=MASK,
- filter_=filter_,
- location=GCP_LOCATION,
- metadata=(),
- page_size=page_size,
- project_id=GCP_PROJECT_ID,
- retry=DEFAULT,
- table_spec_id=table_spec,
- timeout=None,
- )
+ with pytest.raises(AirflowProviderDeprecationWarning, match=self.expected_deprecation_warning):
Review Comment:
@Taragolis in this PR I followed the example of #38673 , which had been approved by the community and merged. In that PR the operators that were no longer working (`AutoMLTablesUpdateDatasetOperator` and `AutoMLDeployModelOperator`) were deprecated with `action="error"` which will raise an exception if we try to instantiate the operators or run their methods.
The operators that I'm trying to deprecate in this PR are also no longer working (the corresponding AutoML functionality was disabled a couple months ago, please read more in the PR desc), so I thought that I should follow the same approach, and assigned `action="error"` in the deprecated decorator.
Because of this `pytest.warns` did not work in the unit tests, so I changed it to `pytest.raises`. I also had to update the related unit tests, because the operator methods started to raise the exceptions instead of working.
In one of your comments you suggest to remove `action="error"` because *if users wanted they could configure it by Python builtin [Warnings Control](https://docs.python.org/3/library/warnings.html#the-warnings-filter) we should not force it to the to the error.*
I can do it, I just think there should be a single approach here. Meaning that:
1) We can follow the example of #38673 , and in this case I don't need to make the changes that you requested.
2) I can follow your suggestions, remove `action="error"` and make the corresponding changes in the unit tests. Then I suppose I will need to do the same for `AutoMLTablesUpdateDatasetOperator` and `AutoMLDeployModelOperator`.
What do you think?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate AutoMLTablesListTableSpecsOperator and AutoMLTablesListColumnSpecsOperator [airflow]
Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis commented on code in PR #39752:
URL: https://github.com/apache/airflow/pull/39752#discussion_r1609608095
##########
airflow/providers/google/cloud/operators/automl.py:
##########
@@ -676,10 +676,25 @@ def execute(self, context: Context):
)
+@deprecated(
+ reason=(
+ "Class `AutoMLTablesListColumnSpecsOperator` has been deprecated and will be removed after 31.12.2024. "
+ "AutoML platform no longer supports tabular datasets after its integration with Cloud Translation and Vertex AI. "
+ "Please refer to https://cloud.google.com/vertex-ai/docs/start/migrating-to-vertex-ai and "
+ "https://cloud.google.com/translate/docs/advanced/automl-upgrade for more info about available alternatives."
+ ),
+ category=AirflowProviderDeprecationWarning,
+ action="error",
Review Comment:
```suggestion
```
If users wanted they could configure it by Python builtin [Warnings Control](https://docs.python.org/3/library/warnings.html#the-warnings-filter) we should not force it to the to the `error`.
##########
airflow/providers/google/cloud/operators/automl.py:
##########
@@ -1182,10 +1197,25 @@ def execute(self, context: Context):
self.log.info("Model was deployed successfully.")
+@deprecated(
+ reason=(
+ "Class `AutoMLTablesListTableSpecsOperator` has been deprecated and will be removed after 31.12.2024. "
+ "AutoML platform no longer supports tabular datasets after its integration with Cloud Translation and Vertex AI. "
+ "Please refer to https://cloud.google.com/vertex-ai/docs/start/migrating-to-vertex-ai and "
+ "https://cloud.google.com/translate/docs/advanced/automl-upgrade for more info about available alternatives."
+ ),
+ category=AirflowProviderDeprecationWarning,
+ action="error",
Review Comment:
```suggestion
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate AutoML Tables operators [airflow]
Posted by "e-galan (via GitHub)" <gi...@apache.org>.
e-galan commented on code in PR #39752:
URL: https://github.com/apache/airflow/pull/39752#discussion_r1613535846
##########
airflow/providers/google/CHANGELOG.rst:
##########
@@ -27,6 +27,20 @@
Changelog
---------
+10.19.0
Review Comment:
Fixed
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate AutoML Tables operators [airflow]
Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis commented on PR #39752:
URL: https://github.com/apache/airflow/pull/39752#issuecomment-2127313589
Is this operators still work in current version of provider?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate AutoML Tables operators [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #39752:
URL: https://github.com/apache/airflow/pull/39752#issuecomment-2128722723
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
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate AutoML Tables operators [airflow]
Posted by "e-galan (via GitHub)" <gi...@apache.org>.
e-galan commented on PR #39752:
URL: https://github.com/apache/airflow/pull/39752#issuecomment-2129495048
Updated CHANGELOG.rst
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate AutoML Tables operators [airflow]
Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis commented on PR #39752:
URL: https://github.com/apache/airflow/pull/39752#issuecomment-2129273839
> Do I need to add some info about the operators to CHANGELOG.rst or will it be added during the release?
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
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate AutoML Tables operators [airflow]
Posted by "e-galan (via GitHub)" <gi...@apache.org>.
e-galan commented on PR #39752:
URL: https://github.com/apache/airflow/pull/39752#issuecomment-2127601400
> When you told "AutoML platform has **deprecated**" you mean "AutoML **removed** / **shutdown** / **some service no longer available** / **discontinued**"?
Yes, I do mean that. Sorry for misunderstanding, but it is the world Google themselves used when describing it, as seen here, for example: https://cloud.google.com/vision/automl/docs/deprecations
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate AutoMLTablesListTableSpecsOperator and AutoMLTablesListColumnSpecsOperator [airflow]
Posted by "e-galan (via GitHub)" <gi...@apache.org>.
e-galan commented on code in PR #39752:
URL: https://github.com/apache/airflow/pull/39752#discussion_r1611230665
##########
tests/providers/google/cloud/operators/test_automl.py:
##########
@@ -379,63 +379,51 @@ def test_templating(self, create_task_instance_of_operator):
assert task.impersonation_chain == "impersonation-chain"
-class TestAutoMLListColumnsSpecsOperator:
+class TestAutoMLTablesListColumnsSpecsOperator:
+ expected_deprecation_warning = (
+ "Class `AutoMLTablesListColumnSpecsOperator` has been deprecated and will be removed after 31.12.2024. "
+ "AutoML platform no longer supports tabular datasets after its integration with Cloud Translation and Vertex AI. "
+ "Please refer to https://cloud.google.com/vertex-ai/docs/start/migrating-to-vertex-ai and "
+ "https://cloud.google.com/translate/docs/advanced/automl-upgrade for more info about available alternatives."
+ )
+
@mock.patch("airflow.providers.google.cloud.operators.automl.CloudAutoMLHook")
def test_execute(self, mock_hook):
table_spec = "table_spec_id"
filter_ = "filter"
page_size = 42
- op = AutoMLTablesListColumnSpecsOperator(
- dataset_id=DATASET_ID,
- table_spec_id=table_spec,
- location=GCP_LOCATION,
- project_id=GCP_PROJECT_ID,
- field_mask=MASK,
- filter_=filter_,
- page_size=page_size,
- task_id=TASK_ID,
- )
- op.execute(context=mock.MagicMock())
- mock_hook.return_value.list_column_specs.assert_called_once_with(
- dataset_id=DATASET_ID,
- field_mask=MASK,
- filter_=filter_,
- location=GCP_LOCATION,
- metadata=(),
- page_size=page_size,
- project_id=GCP_PROJECT_ID,
- retry=DEFAULT,
- table_spec_id=table_spec,
- timeout=None,
- )
+ with pytest.raises(AirflowProviderDeprecationWarning, match=self.expected_deprecation_warning):
Review Comment:
@Taragolis I see that you already responded in #38673 , I will stick to the 2nd approach and refactor all 4 AutoML operators: `AutoMLTablesUpdateDatasetOperator`, `AutoMLDeployModelOperator`, `AutoMLTablesListTableSpecsOperator`, `AutoMLTablesListColumnSpecsOperator`.
Since the 4 operators can no longer be used, as the corresponding AutoML services have reached their EOL in March 2024, it means that it is too late to "deprecate" them normally. We need to tell the users to not use them. So I will remove the `deprecated` decorator completely and add Airflow exceptions into their `__init__` methods.
I hope you are ok with 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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate AutoMLTablesListTableSpecsOperator and AutoMLTablesListColumnSpecsOperator [airflow]
Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis commented on code in PR #39752:
URL: https://github.com/apache/airflow/pull/39752#discussion_r1609551255
##########
tests/providers/google/cloud/operators/test_automl.py:
##########
@@ -379,63 +379,51 @@ def test_templating(self, create_task_instance_of_operator):
assert task.impersonation_chain == "impersonation-chain"
-class TestAutoMLListColumnsSpecsOperator:
+class TestAutoMLTablesListColumnsSpecsOperator:
+ expected_deprecation_warning = (
+ "Class `AutoMLTablesListColumnSpecsOperator` has been deprecated and will be removed after 31.12.2024. "
+ "AutoML platform no longer supports tabular datasets after its integration with Cloud Translation and Vertex AI. "
+ "Please refer to https://cloud.google.com/vertex-ai/docs/start/migrating-to-vertex-ai and "
+ "https://cloud.google.com/translate/docs/advanced/automl-upgrade for more info about available alternatives."
+ )
+
@mock.patch("airflow.providers.google.cloud.operators.automl.CloudAutoMLHook")
def test_execute(self, mock_hook):
table_spec = "table_spec_id"
filter_ = "filter"
page_size = 42
- op = AutoMLTablesListColumnSpecsOperator(
- dataset_id=DATASET_ID,
- table_spec_id=table_spec,
- location=GCP_LOCATION,
- project_id=GCP_PROJECT_ID,
- field_mask=MASK,
- filter_=filter_,
- page_size=page_size,
- task_id=TASK_ID,
- )
- op.execute(context=mock.MagicMock())
- mock_hook.return_value.list_column_specs.assert_called_once_with(
- dataset_id=DATASET_ID,
- field_mask=MASK,
- filter_=filter_,
- location=GCP_LOCATION,
- metadata=(),
- page_size=page_size,
- project_id=GCP_PROJECT_ID,
- retry=DEFAULT,
- table_spec_id=table_spec,
- timeout=None,
- )
Review Comment:
Why this part is removed from tests?
##########
tests/providers/google/cloud/operators/test_automl.py:
##########
@@ -379,63 +379,51 @@ def test_templating(self, create_task_instance_of_operator):
assert task.impersonation_chain == "impersonation-chain"
-class TestAutoMLListColumnsSpecsOperator:
+class TestAutoMLTablesListColumnsSpecsOperator:
+ expected_deprecation_warning = (
+ "Class `AutoMLTablesListColumnSpecsOperator` has been deprecated and will be removed after 31.12.2024. "
+ "AutoML platform no longer supports tabular datasets after its integration with Cloud Translation and Vertex AI. "
+ "Please refer to https://cloud.google.com/vertex-ai/docs/start/migrating-to-vertex-ai and "
+ "https://cloud.google.com/translate/docs/advanced/automl-upgrade for more info about available alternatives."
+ )
+
@mock.patch("airflow.providers.google.cloud.operators.automl.CloudAutoMLHook")
def test_execute(self, mock_hook):
table_spec = "table_spec_id"
filter_ = "filter"
page_size = 42
- op = AutoMLTablesListColumnSpecsOperator(
- dataset_id=DATASET_ID,
- table_spec_id=table_spec,
- location=GCP_LOCATION,
- project_id=GCP_PROJECT_ID,
- field_mask=MASK,
- filter_=filter_,
- page_size=page_size,
- task_id=TASK_ID,
- )
- op.execute(context=mock.MagicMock())
- mock_hook.return_value.list_column_specs.assert_called_once_with(
- dataset_id=DATASET_ID,
- field_mask=MASK,
- filter_=filter_,
- location=GCP_LOCATION,
- metadata=(),
- page_size=page_size,
- project_id=GCP_PROJECT_ID,
- retry=DEFAULT,
- table_spec_id=table_spec,
- timeout=None,
- )
+ with pytest.raises(AirflowProviderDeprecationWarning, match=self.expected_deprecation_warning):
+ _ = AutoMLTablesListColumnSpecsOperator(
+ dataset_id=DATASET_ID,
+ table_spec_id=table_spec,
+ location=GCP_LOCATION,
+ project_id=GCP_PROJECT_ID,
+ field_mask=MASK,
+ filter_=filter_,
+ page_size=page_size,
+ task_id=TASK_ID,
+ )
+ mock_hook.assert_not_called()
@pytest.mark.db_test
def test_templating(self, create_task_instance_of_operator):
- ti = create_task_instance_of_operator(
- AutoMLTablesListColumnSpecsOperator,
- # Templated fields
- dataset_id="{{ 'dataset-id' }}",
- table_spec_id="{{ 'table-spec-id' }}",
- field_mask="{{ 'field-mask' }}",
- filter_="{{ 'filter-' }}",
- location="{{ 'location' }}",
- project_id="{{ 'project-id' }}",
- impersonation_chain="{{ 'impersonation-chain' }}",
- # Other parameters
- dag_id="test_template_body_templating_dag",
- task_id="test_template_body_templating_task",
- execution_date=timezone.datetime(2024, 2, 1, tzinfo=timezone.utc),
- )
- ti.render_templates()
- task: AutoMLTablesListColumnSpecsOperator = ti.task
- assert task.dataset_id == "dataset-id"
- assert task.table_spec_id == "table-spec-id"
- assert task.field_mask == "field-mask"
- assert task.filter_ == "filter-"
- assert task.location == "location"
- assert task.project_id == "project-id"
- assert task.impersonation_chain == "impersonation-chain"
+ with pytest.raises(AirflowProviderDeprecationWarning, match=self.expected_deprecation_warning):
Review Comment:
```suggestion
with pytest.warns(AirflowProviderDeprecationWarning, match=self.expected_deprecation_warning):
```
##########
tests/providers/google/cloud/operators/test_automl.py:
##########
@@ -737,53 +725,45 @@ def test_templating(self, create_task_instance_of_operator):
class TestAutoMLTablesListTableSpecsOperator:
+ expected_deprecation_warning = (
+ "Class `AutoMLTablesListTableSpecsOperator` has been deprecated and will be removed after 31.12.2024. "
+ "AutoML platform no longer supports tabular datasets after its integration with Cloud Translation and Vertex AI. "
+ "Please refer to https://cloud.google.com/vertex-ai/docs/start/migrating-to-vertex-ai and "
+ "https://cloud.google.com/translate/docs/advanced/automl-upgrade for more info about available alternatives."
+ )
+
@mock.patch("airflow.providers.google.cloud.operators.automl.CloudAutoMLHook")
def test_execute(self, mock_hook):
filter_ = "filter"
page_size = 42
- op = AutoMLTablesListTableSpecsOperator(
- dataset_id=DATASET_ID,
- location=GCP_LOCATION,
- project_id=GCP_PROJECT_ID,
- filter_=filter_,
- page_size=page_size,
- task_id=TASK_ID,
- )
- op.execute(context=mock.MagicMock())
- mock_hook.return_value.list_table_specs.assert_called_once_with(
- dataset_id=DATASET_ID,
- filter_=filter_,
- location=GCP_LOCATION,
- metadata=(),
- page_size=page_size,
- project_id=GCP_PROJECT_ID,
- retry=DEFAULT,
- timeout=None,
- )
+ with pytest.raises(AirflowProviderDeprecationWarning, match=self.expected_deprecation_warning):
Review Comment:
```suggestion
with pytest.warns(AirflowProviderDeprecationWarning, match=self.expected_deprecation_warning):
```
##########
tests/providers/google/cloud/operators/test_automl.py:
##########
@@ -737,53 +725,45 @@ def test_templating(self, create_task_instance_of_operator):
class TestAutoMLTablesListTableSpecsOperator:
+ expected_deprecation_warning = (
+ "Class `AutoMLTablesListTableSpecsOperator` has been deprecated and will be removed after 31.12.2024. "
+ "AutoML platform no longer supports tabular datasets after its integration with Cloud Translation and Vertex AI. "
+ "Please refer to https://cloud.google.com/vertex-ai/docs/start/migrating-to-vertex-ai and "
+ "https://cloud.google.com/translate/docs/advanced/automl-upgrade for more info about available alternatives."
+ )
+
@mock.patch("airflow.providers.google.cloud.operators.automl.CloudAutoMLHook")
def test_execute(self, mock_hook):
filter_ = "filter"
page_size = 42
- op = AutoMLTablesListTableSpecsOperator(
- dataset_id=DATASET_ID,
- location=GCP_LOCATION,
- project_id=GCP_PROJECT_ID,
- filter_=filter_,
- page_size=page_size,
- task_id=TASK_ID,
- )
- op.execute(context=mock.MagicMock())
- mock_hook.return_value.list_table_specs.assert_called_once_with(
- dataset_id=DATASET_ID,
- filter_=filter_,
- location=GCP_LOCATION,
- metadata=(),
- page_size=page_size,
- project_id=GCP_PROJECT_ID,
- retry=DEFAULT,
- timeout=None,
- )
Review Comment:
Why this part is removed from tests?
##########
tests/providers/google/cloud/operators/test_automl.py:
##########
@@ -379,63 +379,51 @@ def test_templating(self, create_task_instance_of_operator):
assert task.impersonation_chain == "impersonation-chain"
-class TestAutoMLListColumnsSpecsOperator:
+class TestAutoMLTablesListColumnsSpecsOperator:
+ expected_deprecation_warning = (
+ "Class `AutoMLTablesListColumnSpecsOperator` has been deprecated and will be removed after 31.12.2024. "
+ "AutoML platform no longer supports tabular datasets after its integration with Cloud Translation and Vertex AI. "
+ "Please refer to https://cloud.google.com/vertex-ai/docs/start/migrating-to-vertex-ai and "
+ "https://cloud.google.com/translate/docs/advanced/automl-upgrade for more info about available alternatives."
+ )
+
@mock.patch("airflow.providers.google.cloud.operators.automl.CloudAutoMLHook")
def test_execute(self, mock_hook):
table_spec = "table_spec_id"
filter_ = "filter"
page_size = 42
- op = AutoMLTablesListColumnSpecsOperator(
- dataset_id=DATASET_ID,
- table_spec_id=table_spec,
- location=GCP_LOCATION,
- project_id=GCP_PROJECT_ID,
- field_mask=MASK,
- filter_=filter_,
- page_size=page_size,
- task_id=TASK_ID,
- )
- op.execute(context=mock.MagicMock())
- mock_hook.return_value.list_column_specs.assert_called_once_with(
- dataset_id=DATASET_ID,
- field_mask=MASK,
- filter_=filter_,
- location=GCP_LOCATION,
- metadata=(),
- page_size=page_size,
- project_id=GCP_PROJECT_ID,
- retry=DEFAULT,
- table_spec_id=table_spec,
- timeout=None,
- )
+ with pytest.raises(AirflowProviderDeprecationWarning, match=self.expected_deprecation_warning):
+ _ = AutoMLTablesListColumnSpecsOperator(
+ dataset_id=DATASET_ID,
+ table_spec_id=table_spec,
+ location=GCP_LOCATION,
+ project_id=GCP_PROJECT_ID,
+ field_mask=MASK,
+ filter_=filter_,
+ page_size=page_size,
+ task_id=TASK_ID,
+ )
+ mock_hook.assert_not_called()
@pytest.mark.db_test
def test_templating(self, create_task_instance_of_operator):
- ti = create_task_instance_of_operator(
- AutoMLTablesListColumnSpecsOperator,
- # Templated fields
- dataset_id="{{ 'dataset-id' }}",
- table_spec_id="{{ 'table-spec-id' }}",
- field_mask="{{ 'field-mask' }}",
- filter_="{{ 'filter-' }}",
- location="{{ 'location' }}",
- project_id="{{ 'project-id' }}",
- impersonation_chain="{{ 'impersonation-chain' }}",
- # Other parameters
- dag_id="test_template_body_templating_dag",
- task_id="test_template_body_templating_task",
- execution_date=timezone.datetime(2024, 2, 1, tzinfo=timezone.utc),
- )
- ti.render_templates()
- task: AutoMLTablesListColumnSpecsOperator = ti.task
- assert task.dataset_id == "dataset-id"
- assert task.table_spec_id == "table-spec-id"
- assert task.field_mask == "field-mask"
- assert task.filter_ == "filter-"
- assert task.location == "location"
- assert task.project_id == "project-id"
- assert task.impersonation_chain == "impersonation-chain"
Review Comment:
Why this part is removed from tests?
##########
tests/providers/google/cloud/operators/test_automl.py:
##########
@@ -737,53 +725,45 @@ def test_templating(self, create_task_instance_of_operator):
class TestAutoMLTablesListTableSpecsOperator:
+ expected_deprecation_warning = (
+ "Class `AutoMLTablesListTableSpecsOperator` has been deprecated and will be removed after 31.12.2024. "
+ "AutoML platform no longer supports tabular datasets after its integration with Cloud Translation and Vertex AI. "
+ "Please refer to https://cloud.google.com/vertex-ai/docs/start/migrating-to-vertex-ai and "
+ "https://cloud.google.com/translate/docs/advanced/automl-upgrade for more info about available alternatives."
+ )
+
@mock.patch("airflow.providers.google.cloud.operators.automl.CloudAutoMLHook")
def test_execute(self, mock_hook):
filter_ = "filter"
page_size = 42
- op = AutoMLTablesListTableSpecsOperator(
- dataset_id=DATASET_ID,
- location=GCP_LOCATION,
- project_id=GCP_PROJECT_ID,
- filter_=filter_,
- page_size=page_size,
- task_id=TASK_ID,
- )
- op.execute(context=mock.MagicMock())
- mock_hook.return_value.list_table_specs.assert_called_once_with(
- dataset_id=DATASET_ID,
- filter_=filter_,
- location=GCP_LOCATION,
- metadata=(),
- page_size=page_size,
- project_id=GCP_PROJECT_ID,
- retry=DEFAULT,
- timeout=None,
- )
+ with pytest.raises(AirflowProviderDeprecationWarning, match=self.expected_deprecation_warning):
+ _ = AutoMLTablesListTableSpecsOperator(
+ dataset_id=DATASET_ID,
+ location=GCP_LOCATION,
+ project_id=GCP_PROJECT_ID,
+ filter_=filter_,
+ page_size=page_size,
+ task_id=TASK_ID,
+ )
+ mock_hook.assert_not_called()
@pytest.mark.db_test
def test_templating(self, create_task_instance_of_operator):
- ti = create_task_instance_of_operator(
- AutoMLTablesListTableSpecsOperator,
- # Templated fields
- dataset_id="{{ 'dataset-id' }}",
- filter_="{{ 'filter-' }}",
- location="{{ 'location' }}",
- project_id="{{ 'project-id' }}",
- impersonation_chain="{{ 'impersonation-chain' }}",
- # Other parameters
- dag_id="test_template_body_templating_dag",
- task_id="test_template_body_templating_task",
- execution_date=timezone.datetime(2024, 2, 1, tzinfo=timezone.utc),
- )
- ti.render_templates()
- task: AutoMLTablesListTableSpecsOperator = ti.task
- assert task.dataset_id == "dataset-id"
- assert task.filter_ == "filter-"
- assert task.location == "location"
- assert task.project_id == "project-id"
- assert task.impersonation_chain == "impersonation-chain"
Review Comment:
Why this part is removed from tests?
##########
tests/providers/google/cloud/operators/test_automl.py:
##########
@@ -379,63 +379,51 @@ def test_templating(self, create_task_instance_of_operator):
assert task.impersonation_chain == "impersonation-chain"
-class TestAutoMLListColumnsSpecsOperator:
+class TestAutoMLTablesListColumnsSpecsOperator:
+ expected_deprecation_warning = (
+ "Class `AutoMLTablesListColumnSpecsOperator` has been deprecated and will be removed after 31.12.2024. "
+ "AutoML platform no longer supports tabular datasets after its integration with Cloud Translation and Vertex AI. "
+ "Please refer to https://cloud.google.com/vertex-ai/docs/start/migrating-to-vertex-ai and "
+ "https://cloud.google.com/translate/docs/advanced/automl-upgrade for more info about available alternatives."
+ )
+
@mock.patch("airflow.providers.google.cloud.operators.automl.CloudAutoMLHook")
def test_execute(self, mock_hook):
table_spec = "table_spec_id"
filter_ = "filter"
page_size = 42
- op = AutoMLTablesListColumnSpecsOperator(
- dataset_id=DATASET_ID,
- table_spec_id=table_spec,
- location=GCP_LOCATION,
- project_id=GCP_PROJECT_ID,
- field_mask=MASK,
- filter_=filter_,
- page_size=page_size,
- task_id=TASK_ID,
- )
- op.execute(context=mock.MagicMock())
- mock_hook.return_value.list_column_specs.assert_called_once_with(
- dataset_id=DATASET_ID,
- field_mask=MASK,
- filter_=filter_,
- location=GCP_LOCATION,
- metadata=(),
- page_size=page_size,
- project_id=GCP_PROJECT_ID,
- retry=DEFAULT,
- table_spec_id=table_spec,
- timeout=None,
- )
+ with pytest.raises(AirflowProviderDeprecationWarning, match=self.expected_deprecation_warning):
Review Comment:
Just wondering why you consider to use `pytest.raises` here instead of [`pytest.warns`](https://docs.pytest.org/en/stable/reference/reference.html#pytest-warns), I guess if it failed pytest plugin should suggest to follow https://github.com/apache/airflow/blob/main/contributing-docs/testing/unit_tests.rst#handling-warnings
```suggestion
with pytest.warns(AirflowProviderDeprecationWarning, match=self.expected_deprecation_warning):
```
##########
tests/providers/google/cloud/operators/test_automl.py:
##########
@@ -737,53 +725,45 @@ def test_templating(self, create_task_instance_of_operator):
class TestAutoMLTablesListTableSpecsOperator:
+ expected_deprecation_warning = (
+ "Class `AutoMLTablesListTableSpecsOperator` has been deprecated and will be removed after 31.12.2024. "
+ "AutoML platform no longer supports tabular datasets after its integration with Cloud Translation and Vertex AI. "
+ "Please refer to https://cloud.google.com/vertex-ai/docs/start/migrating-to-vertex-ai and "
+ "https://cloud.google.com/translate/docs/advanced/automl-upgrade for more info about available alternatives."
+ )
+
@mock.patch("airflow.providers.google.cloud.operators.automl.CloudAutoMLHook")
def test_execute(self, mock_hook):
filter_ = "filter"
page_size = 42
- op = AutoMLTablesListTableSpecsOperator(
- dataset_id=DATASET_ID,
- location=GCP_LOCATION,
- project_id=GCP_PROJECT_ID,
- filter_=filter_,
- page_size=page_size,
- task_id=TASK_ID,
- )
- op.execute(context=mock.MagicMock())
- mock_hook.return_value.list_table_specs.assert_called_once_with(
- dataset_id=DATASET_ID,
- filter_=filter_,
- location=GCP_LOCATION,
- metadata=(),
- page_size=page_size,
- project_id=GCP_PROJECT_ID,
- retry=DEFAULT,
- timeout=None,
- )
+ with pytest.raises(AirflowProviderDeprecationWarning, match=self.expected_deprecation_warning):
+ _ = AutoMLTablesListTableSpecsOperator(
+ dataset_id=DATASET_ID,
+ location=GCP_LOCATION,
+ project_id=GCP_PROJECT_ID,
+ filter_=filter_,
+ page_size=page_size,
+ task_id=TASK_ID,
+ )
+ mock_hook.assert_not_called()
@pytest.mark.db_test
def test_templating(self, create_task_instance_of_operator):
- ti = create_task_instance_of_operator(
- AutoMLTablesListTableSpecsOperator,
- # Templated fields
- dataset_id="{{ 'dataset-id' }}",
- filter_="{{ 'filter-' }}",
- location="{{ 'location' }}",
- project_id="{{ 'project-id' }}",
- impersonation_chain="{{ 'impersonation-chain' }}",
- # Other parameters
- dag_id="test_template_body_templating_dag",
- task_id="test_template_body_templating_task",
- execution_date=timezone.datetime(2024, 2, 1, tzinfo=timezone.utc),
- )
- ti.render_templates()
- task: AutoMLTablesListTableSpecsOperator = ti.task
- assert task.dataset_id == "dataset-id"
- assert task.filter_ == "filter-"
- assert task.location == "location"
- assert task.project_id == "project-id"
- assert task.impersonation_chain == "impersonation-chain"
+ with pytest.raises(AirflowProviderDeprecationWarning, match=self.expected_deprecation_warning):
Review Comment:
```suggestion
with pytest.warns(AirflowProviderDeprecationWarning, match=self.expected_deprecation_warning):
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate AutoMLTablesListTableSpecsOperator and AutoMLTablesListColumnSpecsOperator [airflow]
Posted by "e-galan (via GitHub)" <gi...@apache.org>.
e-galan commented on code in PR #39752:
URL: https://github.com/apache/airflow/pull/39752#discussion_r1610768094
##########
tests/providers/google/cloud/operators/test_automl.py:
##########
@@ -379,63 +379,51 @@ def test_templating(self, create_task_instance_of_operator):
assert task.impersonation_chain == "impersonation-chain"
-class TestAutoMLListColumnsSpecsOperator:
+class TestAutoMLTablesListColumnsSpecsOperator:
+ expected_deprecation_warning = (
+ "Class `AutoMLTablesListColumnSpecsOperator` has been deprecated and will be removed after 31.12.2024. "
+ "AutoML platform no longer supports tabular datasets after its integration with Cloud Translation and Vertex AI. "
+ "Please refer to https://cloud.google.com/vertex-ai/docs/start/migrating-to-vertex-ai and "
+ "https://cloud.google.com/translate/docs/advanced/automl-upgrade for more info about available alternatives."
+ )
+
@mock.patch("airflow.providers.google.cloud.operators.automl.CloudAutoMLHook")
def test_execute(self, mock_hook):
table_spec = "table_spec_id"
filter_ = "filter"
page_size = 42
- op = AutoMLTablesListColumnSpecsOperator(
- dataset_id=DATASET_ID,
- table_spec_id=table_spec,
- location=GCP_LOCATION,
- project_id=GCP_PROJECT_ID,
- field_mask=MASK,
- filter_=filter_,
- page_size=page_size,
- task_id=TASK_ID,
- )
- op.execute(context=mock.MagicMock())
- mock_hook.return_value.list_column_specs.assert_called_once_with(
- dataset_id=DATASET_ID,
- field_mask=MASK,
- filter_=filter_,
- location=GCP_LOCATION,
- metadata=(),
- page_size=page_size,
- project_id=GCP_PROJECT_ID,
- retry=DEFAULT,
- table_spec_id=table_spec,
- timeout=None,
- )
+ with pytest.raises(AirflowProviderDeprecationWarning, match=self.expected_deprecation_warning):
Review Comment:
@Taragolis in this PR I followed the example of #38673 , which had been approved by the community and merged. In that PR the operators that were no longer working (`AutoMLTablesUpdateDatasetOperator` and `AutoMLDeployModelOperator`) were deprecated with `action="error"` which will raise an exception when if we try to instantiate the operators or run their methods.
The operators that I'm trying to deprecate in this PR are also no longer working (the corresponding AutoML functionality was disabled a couple months ago, please read more in the PR desc), so I thought that I should follow the same approach, and assigned `action="error"` in the deprecated decorator.
Because of this `pytest.warns` did not work in the unit tests, so I changed it to `pytest.raises`. I also had to update the related unit tests, because the operator methods started to raise the exceptions instead of working.
In one of your comments you suggest to remove `action="error"` because *if users wanted they could configure it by Python builtin [Warnings Control](https://docs.python.org/3/library/warnings.html#the-warnings-filter) we should not force it to the to the error.*
I can do it, I just think there should be a single approach here. Meaning that:
1) We can follow the example of #38673 , and in this case I don't need to make the changes that you requested.
2) I can follow your suggestions, remove `action="error"` and make the corresponding changes in the unit tests. Then I suppose I will need to do the same for `AutoMLTablesUpdateDatasetOperator` and `AutoMLDeployModelOperator`.
What do you think?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate AutoMLTablesListTableSpecsOperator and AutoMLTablesListColumnSpecsOperator [airflow]
Posted by "e-galan (via GitHub)" <gi...@apache.org>.
e-galan commented on code in PR #39752:
URL: https://github.com/apache/airflow/pull/39752#discussion_r1611230665
##########
tests/providers/google/cloud/operators/test_automl.py:
##########
@@ -379,63 +379,51 @@ def test_templating(self, create_task_instance_of_operator):
assert task.impersonation_chain == "impersonation-chain"
-class TestAutoMLListColumnsSpecsOperator:
+class TestAutoMLTablesListColumnsSpecsOperator:
+ expected_deprecation_warning = (
+ "Class `AutoMLTablesListColumnSpecsOperator` has been deprecated and will be removed after 31.12.2024. "
+ "AutoML platform no longer supports tabular datasets after its integration with Cloud Translation and Vertex AI. "
+ "Please refer to https://cloud.google.com/vertex-ai/docs/start/migrating-to-vertex-ai and "
+ "https://cloud.google.com/translate/docs/advanced/automl-upgrade for more info about available alternatives."
+ )
+
@mock.patch("airflow.providers.google.cloud.operators.automl.CloudAutoMLHook")
def test_execute(self, mock_hook):
table_spec = "table_spec_id"
filter_ = "filter"
page_size = 42
- op = AutoMLTablesListColumnSpecsOperator(
- dataset_id=DATASET_ID,
- table_spec_id=table_spec,
- location=GCP_LOCATION,
- project_id=GCP_PROJECT_ID,
- field_mask=MASK,
- filter_=filter_,
- page_size=page_size,
- task_id=TASK_ID,
- )
- op.execute(context=mock.MagicMock())
- mock_hook.return_value.list_column_specs.assert_called_once_with(
- dataset_id=DATASET_ID,
- field_mask=MASK,
- filter_=filter_,
- location=GCP_LOCATION,
- metadata=(),
- page_size=page_size,
- project_id=GCP_PROJECT_ID,
- retry=DEFAULT,
- table_spec_id=table_spec,
- timeout=None,
- )
+ with pytest.raises(AirflowProviderDeprecationWarning, match=self.expected_deprecation_warning):
Review Comment:
@Taragolis I see that you already responded in #38673 , I will stick to the 2nd approach.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate AutoML Tables operators [airflow]
Posted by "e-galan (via GitHub)" <gi...@apache.org>.
e-galan commented on code in PR #39752:
URL: https://github.com/apache/airflow/pull/39752#discussion_r1611707878
##########
tests/providers/google/cloud/operators/test_automl.py:
##########
@@ -379,63 +379,51 @@ def test_templating(self, create_task_instance_of_operator):
assert task.impersonation_chain == "impersonation-chain"
-class TestAutoMLListColumnsSpecsOperator:
+class TestAutoMLTablesListColumnsSpecsOperator:
+ expected_deprecation_warning = (
+ "Class `AutoMLTablesListColumnSpecsOperator` has been deprecated and will be removed after 31.12.2024. "
+ "AutoML platform no longer supports tabular datasets after its integration with Cloud Translation and Vertex AI. "
+ "Please refer to https://cloud.google.com/vertex-ai/docs/start/migrating-to-vertex-ai and "
+ "https://cloud.google.com/translate/docs/advanced/automl-upgrade for more info about available alternatives."
+ )
+
@mock.patch("airflow.providers.google.cloud.operators.automl.CloudAutoMLHook")
def test_execute(self, mock_hook):
table_spec = "table_spec_id"
filter_ = "filter"
page_size = 42
- op = AutoMLTablesListColumnSpecsOperator(
- dataset_id=DATASET_ID,
- table_spec_id=table_spec,
- location=GCP_LOCATION,
- project_id=GCP_PROJECT_ID,
- field_mask=MASK,
- filter_=filter_,
- page_size=page_size,
- task_id=TASK_ID,
- )
- op.execute(context=mock.MagicMock())
- mock_hook.return_value.list_column_specs.assert_called_once_with(
- dataset_id=DATASET_ID,
- field_mask=MASK,
- filter_=filter_,
- location=GCP_LOCATION,
- metadata=(),
- page_size=page_size,
- project_id=GCP_PROJECT_ID,
- retry=DEFAULT,
- table_spec_id=table_spec,
- timeout=None,
- )
Review Comment:
Because `AirflowException` is raised in the operator's `__init__` method. Therefore we cannot instantiate it and run any methods.
##########
tests/providers/google/cloud/operators/test_automl.py:
##########
@@ -379,63 +379,51 @@ def test_templating(self, create_task_instance_of_operator):
assert task.impersonation_chain == "impersonation-chain"
-class TestAutoMLListColumnsSpecsOperator:
+class TestAutoMLTablesListColumnsSpecsOperator:
+ expected_deprecation_warning = (
+ "Class `AutoMLTablesListColumnSpecsOperator` has been deprecated and will be removed after 31.12.2024. "
+ "AutoML platform no longer supports tabular datasets after its integration with Cloud Translation and Vertex AI. "
+ "Please refer to https://cloud.google.com/vertex-ai/docs/start/migrating-to-vertex-ai and "
+ "https://cloud.google.com/translate/docs/advanced/automl-upgrade for more info about available alternatives."
+ )
+
@mock.patch("airflow.providers.google.cloud.operators.automl.CloudAutoMLHook")
def test_execute(self, mock_hook):
table_spec = "table_spec_id"
filter_ = "filter"
page_size = 42
- op = AutoMLTablesListColumnSpecsOperator(
- dataset_id=DATASET_ID,
- table_spec_id=table_spec,
- location=GCP_LOCATION,
- project_id=GCP_PROJECT_ID,
- field_mask=MASK,
- filter_=filter_,
- page_size=page_size,
- task_id=TASK_ID,
- )
- op.execute(context=mock.MagicMock())
- mock_hook.return_value.list_column_specs.assert_called_once_with(
- dataset_id=DATASET_ID,
- field_mask=MASK,
- filter_=filter_,
- location=GCP_LOCATION,
- metadata=(),
- page_size=page_size,
- project_id=GCP_PROJECT_ID,
- retry=DEFAULT,
- table_spec_id=table_spec,
- timeout=None,
- )
+ with pytest.raises(AirflowProviderDeprecationWarning, match=self.expected_deprecation_warning):
+ _ = AutoMLTablesListColumnSpecsOperator(
+ dataset_id=DATASET_ID,
+ table_spec_id=table_spec,
+ location=GCP_LOCATION,
+ project_id=GCP_PROJECT_ID,
+ field_mask=MASK,
+ filter_=filter_,
+ page_size=page_size,
+ task_id=TASK_ID,
+ )
+ mock_hook.assert_not_called()
@pytest.mark.db_test
def test_templating(self, create_task_instance_of_operator):
- ti = create_task_instance_of_operator(
- AutoMLTablesListColumnSpecsOperator,
- # Templated fields
- dataset_id="{{ 'dataset-id' }}",
- table_spec_id="{{ 'table-spec-id' }}",
- field_mask="{{ 'field-mask' }}",
- filter_="{{ 'filter-' }}",
- location="{{ 'location' }}",
- project_id="{{ 'project-id' }}",
- impersonation_chain="{{ 'impersonation-chain' }}",
- # Other parameters
- dag_id="test_template_body_templating_dag",
- task_id="test_template_body_templating_task",
- execution_date=timezone.datetime(2024, 2, 1, tzinfo=timezone.utc),
- )
- ti.render_templates()
- task: AutoMLTablesListColumnSpecsOperator = ti.task
- assert task.dataset_id == "dataset-id"
- assert task.table_spec_id == "table-spec-id"
- assert task.field_mask == "field-mask"
- assert task.filter_ == "filter-"
- assert task.location == "location"
- assert task.project_id == "project-id"
- assert task.impersonation_chain == "impersonation-chain"
Review Comment:
Because `AirflowException` is raised in the operator's `__init__` method. Therefore we cannot instantiate it and run any methods.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate AutoML Tables operators [airflow]
Posted by "e-galan (via GitHub)" <gi...@apache.org>.
e-galan commented on code in PR #39752:
URL: https://github.com/apache/airflow/pull/39752#discussion_r1611709929
##########
airflow/providers/google/cloud/operators/automl.py:
##########
@@ -676,10 +676,25 @@ def execute(self, context: Context):
)
+@deprecated(
+ reason=(
+ "Class `AutoMLTablesListColumnSpecsOperator` has been deprecated and will be removed after 31.12.2024. "
+ "AutoML platform no longer supports tabular datasets after its integration with Cloud Translation and Vertex AI. "
+ "Please refer to https://cloud.google.com/vertex-ai/docs/start/migrating-to-vertex-ai and "
+ "https://cloud.google.com/translate/docs/advanced/automl-upgrade for more info about available alternatives."
+ ),
+ category=AirflowProviderDeprecationWarning,
+ action="error",
Review Comment:
Removed the decorator.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate AutoML Tables operators [airflow]
Posted by "e-galan (via GitHub)" <gi...@apache.org>.
e-galan commented on PR #39752:
URL: https://github.com/apache/airflow/pull/39752#issuecomment-2127281714
> 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
>
> * Finally removed in upstream service / protocol / library
> * We decide that is a good time to release a major version of provider for avoid maintenance hell ( cc: @eladkal Maybe it is a good time to release new major version of Google provider, there is a lot of deprecations from Airflow 1.10)
>
> 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 inform the users that the operators can no longer be supported, and suggests an alternative operator, if it exists.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate AutoML Tables operators [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #39752:
URL: https://github.com/apache/airflow/pull/39752#discussion_r1613519746
##########
airflow/providers/google/CHANGELOG.rst:
##########
@@ -27,6 +27,20 @@
Changelog
---------
+10.19.0
Review Comment:
```suggestion
Main
```
Only during release we set the version
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate AutoML Tables operators [airflow]
Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis commented on PR #39752:
URL: https://github.com/apache/airflow/pull/39752#issuecomment-2127624540
Yeah I've already I found information: https://cloud.google.com/vision/automl/docs/deprecations
- Service Deprecated from January 23, 2023
- Service Shutdown from March 31, 2024
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](https://peps.python.org/pep-0562/)
```python
# 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
```text
Operator/Hook/Sensor/Class FooBar has been removed due to shutdown a legacy version of AutoML Vision on March 31, 2024.
see: https://cloud.google.com/vision/automl/docs/deprecations
Suggestion to switch to another Operator/Hook/Sensor/Class if there is any exists or steps what end users could also do.
```
And finally we should add information about removals in provider [CHANGELOG.rst](https://github.com/apache/airflow/blob/97e867f1701fae287a20dff7e0f9e9ecbdc38f27/airflow/providers/google/CHANGELOG.rst).
https://github.com/apache/airflow/blob/97e867f1701fae287a20dff7e0f9e9ecbdc38f27/airflow/providers/google/CHANGELOG.rst?plain=1#L229-L237
https://github.com/apache/airflow/blob/fe4605a10e26f1b8a180979ba5765d1cb7fb0111/airflow/providers/slack/CHANGELOG.rst?plain=1#L37-L52
https://github.com/apache/airflow/blob/97e867f1701fae287a20dff7e0f9e9ecbdc38f27/airflow/providers/microsoft/azure/CHANGELOG.rst?plain=1#L458-L463
The same valid for the operators with already removed functional but it could be done by the separate PR
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate AutoML Tables operators [airflow]
Posted by "e-galan (via GitHub)" <gi...@apache.org>.
e-galan commented on PR #39752:
URL: https://github.com/apache/airflow/pull/39752#issuecomment-2129161984
@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](https://github.com/apache/airflow/blob/97e867f1701fae287a20dff7e0f9e9ecbdc38f27/airflow/providers/google/CHANGELOG.rst) or will it be added during the release?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate AutoML Tables operators [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal merged PR #39752:
URL: https://github.com/apache/airflow/pull/39752
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate AutoML Tables operators [airflow]
Posted by "e-galan (via GitHub)" <gi...@apache.org>.
e-galan commented on code in PR #39752:
URL: https://github.com/apache/airflow/pull/39752#discussion_r1611709375
##########
airflow/providers/google/cloud/operators/automl.py:
##########
@@ -1182,10 +1197,25 @@ def execute(self, context: Context):
self.log.info("Model was deployed successfully.")
+@deprecated(
+ reason=(
+ "Class `AutoMLTablesListTableSpecsOperator` has been deprecated and will be removed after 31.12.2024. "
+ "AutoML platform no longer supports tabular datasets after its integration with Cloud Translation and Vertex AI. "
+ "Please refer to https://cloud.google.com/vertex-ai/docs/start/migrating-to-vertex-ai and "
+ "https://cloud.google.com/translate/docs/advanced/automl-upgrade for more info about available alternatives."
+ ),
+ category=AirflowProviderDeprecationWarning,
+ action="error",
Review Comment:
Removed the decorator.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate AutoML Tables operators [airflow]
Posted by "e-galan (via GitHub)" <gi...@apache.org>.
e-galan commented on code in PR #39752:
URL: https://github.com/apache/airflow/pull/39752#discussion_r1611708439
##########
tests/providers/google/cloud/operators/test_automl.py:
##########
@@ -737,53 +725,45 @@ def test_templating(self, create_task_instance_of_operator):
class TestAutoMLTablesListTableSpecsOperator:
+ expected_deprecation_warning = (
+ "Class `AutoMLTablesListTableSpecsOperator` has been deprecated and will be removed after 31.12.2024. "
+ "AutoML platform no longer supports tabular datasets after its integration with Cloud Translation and Vertex AI. "
+ "Please refer to https://cloud.google.com/vertex-ai/docs/start/migrating-to-vertex-ai and "
+ "https://cloud.google.com/translate/docs/advanced/automl-upgrade for more info about available alternatives."
+ )
+
@mock.patch("airflow.providers.google.cloud.operators.automl.CloudAutoMLHook")
def test_execute(self, mock_hook):
filter_ = "filter"
page_size = 42
- op = AutoMLTablesListTableSpecsOperator(
- dataset_id=DATASET_ID,
- location=GCP_LOCATION,
- project_id=GCP_PROJECT_ID,
- filter_=filter_,
- page_size=page_size,
- task_id=TASK_ID,
- )
- op.execute(context=mock.MagicMock())
- mock_hook.return_value.list_table_specs.assert_called_once_with(
- dataset_id=DATASET_ID,
- filter_=filter_,
- location=GCP_LOCATION,
- metadata=(),
- page_size=page_size,
- project_id=GCP_PROJECT_ID,
- retry=DEFAULT,
- timeout=None,
- )
Review Comment:
Because `AirflowException` is raised in the operator's `__init__` method. Therefore we cannot instantiate it and run any methods.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate AutoML Tables operators [airflow]
Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis commented on PR #39752:
URL: https://github.com/apache/airflow/pull/39752#issuecomment-2127256640
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
- Finally removed in upstream service / protocol / library
- We decide that is a good time to release a major version of provider for avoid maintenance hell ( cc: @eladkal Maybe it is
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"
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org