Skip to content

Deprecation of AutoML services: Add deprecation warnings and raise exceptions for already deprecated ones #38673

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.

Sign up for GitHub

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

Jump to bottom
Merged
potiuk merged 1 commit into from
May 10, 2024
Merged

Deprecation of AutoML services: Add deprecation warnings and raise exceptions for already deprecated ones #38673

potiuk merged 1 commit into from
May 10, 2024

Conversation

@molcay
Copy link
Contributor

@molcay molcay commented Apr 2, 2024 •
edited
Loading

Before merging this PR we need to merge #38633

This PR is for raising exception for the deprecated services of AutoML (Tables, Vision, Video Intelligence, Natural Language) which are EoL or will be EoL soon.

^ 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.

@boring-cyborg boring-cyborg bot added area:providers provider:google Google (including GCP) related issues labels Apr 2, 2024
@molcay molcay changed the title Add deprecation warnings and raise exception for already deprecated ones Deprecation of AutoML services: Add deprecation warnings and raise exceptions for already deprecated ones Apr 2, 2024
@molcay molcay mentioned this pull request Apr 2, 2024
Merged
@shahar1 shahar1 mentioned this pull request Apr 2, 2024
Closed
@eladkal eladkal force-pushed the deprecate-automl branch from ee79945 to 2aacd97 Compare April 2, 2024 17:13
eladkal
eladkal reviewed Apr 2, 2024
View reviewed changes
@potiuk potiuk force-pushed the deprecate-automl branch from 2aacd97 to 12e0d44 Compare April 2, 2024 19:25
@molcay molcay force-pushed the deprecate-automl branch from 12e0d44 to 9f87f76 Compare April 4, 2024 02:01
@molcay molcay requested review from ashb and potiuk as code owners April 4, 2024 02:01
@molcay molcay force-pushed the deprecate-automl branch 3 times, most recently from f75a35e to 5331569 Compare April 5, 2024 08:29
@potiuk
Copy link
Member

potiuk commented Apr 5, 2024

I think you need to rebase the PR. It's 52 commits behind and this is likely why it fails

@molcay molcay force-pushed the deprecate-automl branch 3 times, most recently from 8d86abd to 338ca37 Compare April 9, 2024 08:34
Taragolis
Taragolis reviewed Apr 9, 2024
View reviewed changes
shahar1
shahar1 requested changes Apr 9, 2024
View reviewed changes
Copy link
Contributor

@shahar1 shahar1 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Good start!
There's some work to do to finish it up (mostly adding tests and docstrings), and I hope that my comments will make sense - please ask if you have any questions.
I'd be happy if you could test the operator against an actual GCP project with enabled AutoML API to see if all functionalities work as expected :)

@molcay molcay force-pushed the deprecate-automl branch 3 times, most recently from 9f17725 to 1554e19 Compare April 15, 2024 09:18
@potiuk
Copy link
Member

potiuk commented Apr 23, 2024

@molcay -> can you please rebase/resolve conflict and respond/mark as resolved all conversations that you think were resolved?

@molcay molcay force-pushed the deprecate-automl branch 2 times, most recently from c646a71 to ec88665 Compare April 24, 2024 14:54
@potiuk
Copy link
Member

potiuk commented Apr 24, 2024

cc: @VladaZakharova - there were some doubts for Google team about those deprecations - does this one look ok for you?

@molcay molcay force-pushed the deprecate-automl branch from ec88665 to 546199a Compare April 25, 2024 07:45
@VladaZakharova
Copy link
Contributor

VladaZakharova commented Apr 25, 2024

cc: @VladaZakharova - there were some doubts for Google team about those deprecations - does this one look ok for you?

Hi! Thanks, yes, changes look good, LGTM

@potiuk
Copy link
Member

potiuk commented Apr 25, 2024

@molcay -> can you please review and address/mark as resolved if they are - the comments made by @shahar1 ?

@molcay
Copy link
Contributor Author

molcay commented Apr 26, 2024

@potiuk,
Ohh I see, there are some hidden conversations. I totally missed them. Sorry for the inconvenience. I will try to address and resolve them as soon as possible.

@molcay molcay force-pushed the deprecate-automl branch from da243a1 to c7748ae Compare April 30, 2024 14:39
This was referenced Apr 30, 2024
Closed
Closed
shahar1
shahar1 approved these changes May 1, 2024
View reviewed changes
Copy link
Contributor

@shahar1 shahar1 left a comment

Choose a reason for hiding this comment

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

Very good progress! :)
You resolved all of my comments, so you get a sincere approval from me.
One last step before merging - try to rebase and ensure that tests/providers/google/cloud/operators/test_automl.py passes in the Non-DB tests, as currently they fail on the CI.
Make sure that objects are mocked properly in tests that you added or modified.

@molcay
Copy link
Contributor Author

molcay commented May 2, 2024

I will work on this test failure and send an update. Thank you for the review @shahar1
Also, thank you for pinging and guiding along the way @potiuk

@molcay molcay force-pushed the deprecate-automl branch 2 times, most recently from 57b7fc4 to fec27f3 Compare May 2, 2024 09:07
@eladkal eladkal force-pushed the deprecate-automl branch from fec27f3 to 5191583 Compare May 7, 2024 14:51
@molcay molcay force-pushed the deprecate-automl branch from 5191583 to bb2bbc8 Compare May 9, 2024 09:26
@VladaZakharova
Copy link
Contributor

VladaZakharova commented May 9, 2024

Hey @shahar1 !
Would be nice if you can check the changes here again

@eladkal eladkal requested a review from shahar1 May 10, 2024 05:42
shahar1
shahar1 approved these changes May 10, 2024
View reviewed changes
Copy link
Contributor

@shahar1 shahar1 left a comment

Choose a reason for hiding this comment

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

Re-reviewed, we're good to go :)

@potiuk potiuk merged commit 8dcee5b into apache:main May 10, 2024
@eladkal eladkal mentioned this pull request May 12, 2024
Closed
66 tasks
pateash pushed a commit to pateash/airflow that referenced this pull request May 13, 2024
@molcay @pateash
Add deprecation warnings and raise exception for already deprecated o…
9108133 
…nes (apache#38673)
This was referenced May 16, 2024
Merged
Merged
@Taragolis Taragolis mentioned this pull request May 22, 2024
Closed
@eladkal eladkal mentioned this pull request May 26, 2024
Closed
@utkarsharma2 utkarsharma2 added type:improvement Changelog: Improvements changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) and removed type:improvement Changelog: Improvements labels Jun 3, 2024
romsharon98 pushed a commit to romsharon98/airflow that referenced this pull request Jul 26, 2024
@molcay @romsharon98
Add deprecation warnings and raise exception for already deprecated o…
0ba1113 
…nes (apache#38673)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Taragolis Taragolis Taragolis left review comments

@potiuk potiuk potiuk approved these changes

@eladkal eladkal eladkal approved these changes

@shahar1 shahar1 shahar1 approved these changes

@ashb ashb Awaiting requested review from ashb ashb is a code owner

+1 more reviewer

@VladaZakharova VladaZakharova VladaZakharova approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area:providers changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) provider:google Google (including GCP) related issues

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@molcay @potiuk @VladaZakharova @Taragolis @eladkal @shahar1 @utkarsharma2