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

Add Iceberg provider #39155

Merged
merged 29 commits into from
May 10, 2024
Merged

Add Iceberg provider #39155

merged 29 commits into from
May 10, 2024

Conversation

romsharon98
Copy link
Contributor

@romsharon98 romsharon98 commented Apr 21, 2024

Following #32786 (comment) since Tabular provider doesn't have any specific tabular logic it has been suggested to move Tabular logic to new Apache Iceberg provider


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

@eladkal
Copy link
Contributor

eladkal commented Apr 21, 2024

@Fokko Do we want to keep Tabular provider (if there might be future tabular specific behavior?) or should we deprecate the whole tabular provider in favor of apache-iceberg provider? (difference between deprecate specific classes vs the whole provider)

@romsharon98 romsharon98 self-assigned this Apr 23, 2024
@Fokko
Copy link
Contributor

Fokko commented Apr 29, 2024

@Fokko Do we want to keep Tabular provider (if there might be future tabular specific behavior?) or should we deprecate the whole tabular provider in favor of apache-iceberg provider? (difference between deprecate specific classes vs the whole provider)

No, let's deprecate it in favor of the Iceberg provider. Everything in here is just Iceberg, and nothing Tabular specific, so the current naming is confusion

@Fokko
Copy link
Contributor

Fokko commented Apr 29, 2024

@romsharon98 Sorry for being late to the party here, but this looks great! Thanks for picking this up 🎉

@romsharon98 romsharon98 force-pushed the feature/iceberg-provider branch from e8b8d3e to 4726ce3 Compare April 30, 2024 10:53
@romsharon98 romsharon98 marked this pull request as ready for review April 30, 2024 17:52
@romsharon98 romsharon98 force-pushed the feature/iceberg-provider branch from 45366db to c06823b Compare April 30, 2024 17:52
Copy link
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

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

Some small comments, but this looks great @romsharon98

@romsharon98 romsharon98 force-pushed the feature/iceberg-provider branch from c06823b to 9710a4e Compare May 1, 2024 07:17
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.

Marking as request changes just to wait for my review and after release of current provider wave
I'll get to it in 1-2 days

@eladkal eladkal changed the title Feature/iceberg provider Add Iceberg provider May 1, 2024
@Fokko
Copy link
Contributor

Fokko commented May 1, 2024

@eladkal That makes sense. It was not my intent to merge this without your approval, but just to indicate that there are no further comments on the Operator and Hook itself.

@eladkal
Copy link
Contributor

eladkal commented May 1, 2024

@eladkal That makes sense. It was not my intent to merge this without your approval

I know :)
This is just to avoid merge by another committer. We are 99% ready here. it's just for the procedure of release due to #39240
for simplicity I will handle it as ad-hoc release

@romsharon98 romsharon98 force-pushed the feature/iceberg-provider branch from fc0a936 to b352f39 Compare May 7, 2024 09:54
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.

LGTM
will merge when CI is green

@romsharon98 romsharon98 force-pushed the feature/iceberg-provider branch from 3fa715a to a81d839 Compare May 9, 2024 20:36
@eladkal eladkal merged commit da79f6b into apache:main May 10, 2024
91 checks passed
@eladkal
Copy link
Contributor

eladkal commented May 10, 2024

🎉🎉🎉

@potiuk
Copy link
Member

potiuk commented May 10, 2024

Wooohooo!

@Fokko
Copy link
Contributor

Fokko commented May 10, 2024

Nice! Thanks @romsharon98 for working on this, and thanks @eladkal and @potiuk for the review 🎉

jedcunningham added a commit to astronomer/airflow that referenced this pull request May 11, 2024
When the tabular provider was deprecated in favor of iceberg in apache#39155,
it appears there was a bad merge conflict resolution that wiped out the
existence of 1.5.0.
pateash pushed a commit to pateash/airflow that referenced this pull request May 13, 2024
* add log for running callback

* revert

* add iceberg provider

* add tabular deprecation

* fix comment

* fix comment

* fix redirect

* deprecated tabular, fix integration name

* fix connections.rst

* merge with main

* remove iceberg default host

* add to providers bug report

* fix changelog and revert __init__

* remove redirects, remove tabular new version, revert latest docs only

* remove deperecated hook

* add deprecation warning

* revert tabular connection and add iceberg connection

* fix iceberg tests

* fix iceberg connection test

* fix iceberg connection

* mock the correct connection

* tabular should not have tests

* remove deprecated hook from yaml

* fix integration name

* add iceberg logo

* fix integrations

* fix iceberg logo location

* revert tabular in extra-packages-ref

* fix docs
@utkarsharma2 utkarsharma2 added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Jun 3, 2024
romsharon98 added a commit to romsharon98/airflow that referenced this pull request Jul 26, 2024
* add log for running callback

* revert

* add iceberg provider

* add tabular deprecation

* fix comment

* fix comment

* fix redirect

* deprecated tabular, fix integration name

* fix connections.rst

* merge with main

* remove iceberg default host

* add to providers bug report

* fix changelog and revert __init__

* remove redirects, remove tabular new version, revert latest docs only

* remove deperecated hook

* add deprecation warning

* revert tabular connection and add iceberg connection

* fix iceberg tests

* fix iceberg connection test

* fix iceberg connection

* mock the correct connection

* tabular should not have tests

* remove deprecated hook from yaml

* fix integration name

* add iceberg logo

* fix integrations

* fix iceberg logo location

* revert tabular in extra-packages-ref

* fix docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants