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
Collaborator

@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 marked this pull request as ready for review 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

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

airflow/providers/tabular/CHANGELOG.rst Outdated Show resolved Hide resolved
airflow/providers/tabular/CHANGELOG.rst Outdated Show resolved Hide resolved
airflow/providers/tabular/__init__.py Outdated Show resolved Hide resolved
docs/apache-airflow-providers-tabular/changelog.rst Outdated Show resolved Hide resolved
docs/apache-airflow-providers-tabular/redirects.txt Outdated Show resolved Hide resolved
airflow/providers/tabular/CHANGELOG.rst Outdated Show resolved Hide resolved
airflow/providers/tabular/provider.yaml Outdated Show resolved Hide resolved
airflow/providers/tabular/hooks/tabular.py Show resolved Hide resolved
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

@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 🎉

RodrigoGanancia pushed a commit to RodrigoGanancia/airflow that referenced this pull request May 10, 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
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
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.

None yet

5 participants