[FLINK-30064] Move existing Hive connector code from Flink repo to dedicated Hive repo #5

snuyanzin · 2023-11-16T01:13:02Z

The PR is to move hive connector to external repo

history was taken based on https://cwiki.apache.org/confluence/display/FLINK/Externalized+Connector+development#ExternalizedConnectordevelopment-Githistory

boring-cyborg · 2023-11-16T01:13:07Z

Thanks for opening this pull request! Please check out our contributing guidelines. (https://flink.apache.org/contributing/how-to-contribute.html)

MartijnVisser · 2023-11-16T07:56:04Z

.github/workflows/push_pr.yml

@@ -25,4 +25,4 @@ jobs:
  compile_and_test:
    uses: apache/flink-connector-shared-utils/.github/workflows/ci.yml@ci_utils
    with:
-      flink_version: 1.16.0
+      flink_version: 1.19-SNAPSHOT


Is the externalization based on the code from the release-1.18 branch or master?

initial commit is based on master however let's see if there could be done support for 1.18.0
there are several things which are required to be workaround like FLINK-32620, FLINK-25593 and some others

yes, with last 2 commits 1.18 is supported.

there could be 2 ways: support it for 1.18.0 first only and then apply similar approach to adapt updates from 1.19
or support it for both 1.19 and 1.18 like here

snuyanzin · 2023-11-25T16:18:57Z

rebased since there were new commits in main flink master to hive-connector area

RyanSkraba

Hello Sergey -- sorry this took so long to review! In principle, it looks pretty good, but I have a question about the migration of test utilities from

flink: ./flink-end-to-end-tests/flink-end-to-end-tests-common/src/main to
flink-connector-hive: /flink-connector-hive-e2e-tests/src/main/

The migrated package is org.apache.flink.tests.util.flink but other connectors use the form org.apache.link.tests.util.kafka for example.
Just for info, are these utilities unique to the flink Hive connector?

There's some weirdness going on with my local build, where I can't seem to get the docker images in the TestContainers to run, but it's hard to argue with the CI passing! I'm investigating the reason locally.

Thanks for this work!!
These files are necessary for the flink end-to-end test

luoyuxia · 2023-12-02T03:39:04Z

Cool! @snuyanzin Thanks for the pr. Sorry for the late response for I'm busy with other things. I'll definitely have a look in next week.

snuyanzin · 2023-12-03T19:01:27Z

rebased since there were new commits in main flink master to hive-connector area

also added archunit violations because of recent commit within FLINK-33637
@RyanSkraba moved test packages to flink.connectors.hive

luoyuxia

@snuyanzin Thanks for the great work. Overall look good to me assuming ci test pass for Hive2 & Hive3.

flink-connector-hive/src/test/java/org/apache/flink/connectors/hive/HiveTableSinkITCase.java

luoyuxia · 2023-12-06T03:05:00Z

@snuyanzin Hi, I also found some pages(https://nightlies.apache.org/flink/flink-docs-master/docs/dev/table/hive-compatibility/hive-dialect/overview/ , https://nightlies.apache.org/flink/flink-docs-master/docs/dev/table/sql-gateway/hiveserver2/) in Flink still refer to Hive with internal link. Will we adjust them in the pr of removing Hive connector from flink repo?

snuyanzin · 2023-12-06T22:13:04Z

Thanks for the feedback

Will we adjust them in the pr of removing Hive connector from flink repo?

yep, usually it's happening either with while removal of connector from main repo or together with the first oficial release of externalised connector.

MartijnVisser

LGTM

luoyuxia

@snuyanzin Thanks for the update. LGTM. Btw, will we also enable test with hive3 nightly just like flink does? I want to make sure we can have a chance to test & verify it with hive3.

MartijnVisser · 2023-12-07T12:23:54Z

I want to make sure we can have a chance to test & verify it with hive3.

Good point. I actually think we should consider running them both for each PR, that should be easier with Hive being externalized.

snuyanzin · 2023-12-07T12:38:07Z

agree, makes sense
will try do that
however currently this is no supported by flink-connector-shared-utils ...

the only solution is to copy paste ci.yml and enable running tests against different hive versions

I will see if it could be supported on a generic way since same functionality would be nice to have for OpenSearch connector

MartijnVisser · 2023-12-07T12:46:38Z

I will see if it could be supported on a generic way since same functionality would be nice to have for OpenSearch connector

We could consider adding optional_maven_profiles as a generic variable and pass that through (kind of like https://github.com/apache/flink-connector-shared-utils/blob/ci_utils/.github/workflows/ci.yml#L87-L89)

snuyanzin · 2023-12-07T13:32:43Z

yep, that exactly the idea i'm testing right now =)
thanks for confirmation 👍

snuyanzin · 2023-12-07T14:01:33Z

just another note, I noticed that hive3 is tested only for jdk8 in flink nightlies
I tried to run it with jdk 11 and e2e failed while for hive2 it is passing
Since it was not run with jdk11 i will add support only for jdk8 for hive3 as it is done in flink main repo
other jdk support could be done separately

snuyanzin · 2023-12-07T20:34:55Z

Currently it works for hive2 and hive3 (without e2e for jdk11)
as

hive version	flink 1.18 + jdk 8	flink 1.19-snapshot + jdk8	flink 1.18 + jdk 11	flink 1.19-snapshot + jdk11
2	✅	✅	✅	✅
3	✅	✅	no E2E	no E2E

as mentioned above under flink main there is no E2E tests for jdk11 as well

The PR currently copies ci.yaml from flink-connector-shared-utils ...

at the same time there are 2 PRs for that repo to support customization of jdk version apache/flink-connector-shared-utils#24 and maven profiles apache/flink-connector-shared-utils#29 , these 2 features are used here. Once they are merged and a new release released ci.yml could be reused and the copy could be removed

@luoyuxia , @MartijnVisser please let me know whether you are ok with this or not

luoyuxia · 2023-12-08T01:35:34Z

@snuyanzin Cool! Thanks for your work. It sounds good to me.

MartijnVisser · 2023-12-08T08:25:52Z

please let me know whether you are ok with this or not

I'm reviewing the CI PRs; when they are merged and we can merge this, will we remove the Hive code from Flink's master branch?

snuyanzin · 2023-12-08T08:38:13Z

I'm reviewing the CI PRs; when they are merged and we can merge this

thanks a lot for your review

will we remove the Hive code from Flink's master branch?

yes, that was the plan, the only question is whether we need a dedicated ML thread for that or since there is already existing jira issue for Hive externalisation, we could just continue with it?

Also I would guess we need to release this connector

…nnector-hive (#23166)

…de (#23267)

… using Hive dialect This closes #23309

This closes #23370

…iguration parameters) to RichFunction#open(OpenContext openContext) This closes #23058

…e push down contains none-existent partition (#23423)

…tion with Hive dialect

…rs/flink-connector-hive (TableTestBase)

…23682)

…eTemporalJoinHiveTable nightly CI fails

…rs (StreamingTestBase)

…ban Junit4 for table-planner module This closes #23791.

…onment#toAppendStream

snuyanzin · 2023-12-08T18:15:56Z

After merging of mentioned PRs to flink-connector-shared-utils all related WAs are removed and ci_utils is reused.

Also there are a couple of commits to hive connector in main repo, one of which leaded to conflicts. Now they are integrated here as well

The PR also enables manual trigger of nightly however it will be possible to trigger this only after merge to main
I will trigger it on my own fork

I'm going to merge it soon after the build is green and nightly on my fork is also green

snuyanzin · 2023-12-08T21:04:53Z

weekly passed
https://github.com/snuyanzin/flink-connector-hive/actions/runs/7145518824

boring-cyborg · 2023-12-08T21:06:59Z

Awesome work, congrats on your first merged pull request!

boring-cyborg bot added component=BuildSystem component=Connectors/Hive component=Documentation labels Nov 16, 2023

MartijnVisser requested review from luoyuxia and MartijnVisser November 16, 2023 07:52

MartijnVisser reviewed Nov 16, 2023

View reviewed changes

snuyanzin force-pushed the flink30064 branch from 9eeb4d8 to f706726 Compare November 25, 2023 15:41

RyanSkraba reviewed Nov 30, 2023

View reviewed changes

RyanSkraba approved these changes Dec 1, 2023

View reviewed changes

snuyanzin force-pushed the flink30064 branch from f706726 to f4379fc Compare December 3, 2023 17:30

luoyuxia reviewed Dec 6, 2023

View reviewed changes

flink-connector-hive/src/test/java/org/apache/flink/connectors/hive/HiveTableSinkITCase.java Show resolved Hide resolved

snuyanzin requested review from luoyuxia and MartijnVisser December 6, 2023 22:13

MartijnVisser approved these changes Dec 7, 2023

View reviewed changes

luoyuxia approved these changes Dec 7, 2023

View reviewed changes

JingGe and others added 20 commits December 8, 2023 15:59

Update version to 1.19-SNAPSHOT

85d7729

[FLINK-32760][Connectors/Hive] Reshade shaded parquet in flink-sql-co…

44efc62

…nnector-hive (#23166)

[FLINK-32731][e2e] Add retry mechanism when fails to start the nameno…

b899b5a

…de (#23267)

[FLINK-32982][doc] Not to suggest to swap flink-table-planner lib for…

13ca975

… using Hive dialect This closes #23309

[FLINK-32620][sink] Migrate DiscardingSink to sinkv2.

7f71579

[FLINK-32731][e2e] Fix NameNode uses port 9870 in hadoop3

77a9bd4

This closes #23370

[FLINK-32978][flink-core] Migrate the usage of RichFunction#open(Conf…

634265b

…iguration parameters) to RichFunction#open(OpenContext openContext) This closes #23058

[FLINK-32223][runtime][security] Add Hive delegation token support

14234e2

[FLINK-25593][table-planner] Skip redundant scan while partition tabl…

bec407e

…e push down contains none-existent partition (#23423)

[FLINK-33233][hive] Fix NPE when non-native udf is used in join condi…

7c75e69

…tion with Hive dialect

[FLINK-33226][table] Forbid to drop current database

348503d

[FLINK-33023][table-planner][JUnit5 Migration] Module: flink-connecto…

91a7419

…rs/flink-connector-hive (TableTestBase)

[FLINK-33478][hive] Enable foldExpr by default in filter condition (#…

4e3f699

…23682)

[FLINK-33551][Connectors/Hive] Fix HiveTemporalJoinITCase.testProcTim…

43491d0

…eTemporalJoinHiveTable nightly CI fails

[hotfix] Remove Calcite classes from Hive connector

7ceae5e

[FLINK-32997][table-planner][JUnit5 Migration] Module: flink-connecto…

58f0f4f

…rs (StreamingTestBase)

[FLINK-33418][test] Uses getHost()

079016f

[FLINK-33637][table-planner][JUnit5 Migration] Introduce ArchTest to …

aa20473

…ban Junit4 for table-planner module This closes #23791.

[hotfix][connector-hive][test] fix ArchUnit typo in pom

99818dd

[FLINK-33718][table] Cleanup the usage of deprecated StreamTableEnvir…

785660f

…onment#toAppendStream

snuyanzin force-pushed the flink30064 branch 2 times, most recently from 5be7963 to 124d9cf Compare December 8, 2023 18:07

[FLINK-30064][hive] Move Hive connector to external repo

13424e8

snuyanzin force-pushed the flink30064 branch from 124d9cf to b4e8d5d Compare December 8, 2023 18:50

[hotfix] Allow manual trigger for weekly

557a369

snuyanzin force-pushed the flink30064 branch from b4e8d5d to 557a369 Compare December 8, 2023 20:33

snuyanzin merged commit 557a369 into apache:main Dec 8, 2023
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FLINK-30064] Move existing Hive connector code from Flink repo to dedicated Hive repo #5

[FLINK-30064] Move existing Hive connector code from Flink repo to dedicated Hive repo #5

snuyanzin commented Nov 16, 2023

boring-cyborg bot commented Nov 16, 2023

MartijnVisser Nov 16, 2023

snuyanzin Nov 16, 2023 •

edited

Loading

snuyanzin Nov 16, 2023

snuyanzin commented Nov 25, 2023 •

edited

Loading

RyanSkraba left a comment

luoyuxia commented Dec 2, 2023

snuyanzin commented Dec 3, 2023 •

edited

Loading

luoyuxia left a comment •

edited

Loading

luoyuxia commented Dec 6, 2023

snuyanzin commented Dec 6, 2023 •

edited

Loading

MartijnVisser left a comment

luoyuxia left a comment

MartijnVisser commented Dec 7, 2023

snuyanzin commented Dec 7, 2023 •

edited

Loading

MartijnVisser commented Dec 7, 2023

snuyanzin commented Dec 7, 2023

snuyanzin commented Dec 7, 2023

snuyanzin commented Dec 7, 2023 •

edited

Loading

luoyuxia commented Dec 8, 2023

MartijnVisser commented Dec 8, 2023

snuyanzin commented Dec 8, 2023 •

edited

Loading

snuyanzin commented Dec 8, 2023

snuyanzin commented Dec 8, 2023

boring-cyborg bot commented Dec 8, 2023

[FLINK-30064] Move existing Hive connector code from Flink repo to dedicated Hive repo #5

[FLINK-30064] Move existing Hive connector code from Flink repo to dedicated Hive repo #5

Conversation

snuyanzin commented Nov 16, 2023

boring-cyborg bot commented Nov 16, 2023

MartijnVisser Nov 16, 2023

Choose a reason for hiding this comment

snuyanzin Nov 16, 2023 • edited Loading

Choose a reason for hiding this comment

snuyanzin Nov 16, 2023

Choose a reason for hiding this comment

snuyanzin commented Nov 25, 2023 • edited Loading

RyanSkraba left a comment

Choose a reason for hiding this comment

luoyuxia commented Dec 2, 2023

snuyanzin commented Dec 3, 2023 • edited Loading

luoyuxia left a comment • edited Loading

Choose a reason for hiding this comment

luoyuxia commented Dec 6, 2023

snuyanzin commented Dec 6, 2023 • edited Loading

MartijnVisser left a comment

Choose a reason for hiding this comment

luoyuxia left a comment

Choose a reason for hiding this comment

MartijnVisser commented Dec 7, 2023

snuyanzin commented Dec 7, 2023 • edited Loading

MartijnVisser commented Dec 7, 2023

snuyanzin commented Dec 7, 2023

snuyanzin commented Dec 7, 2023

snuyanzin commented Dec 7, 2023 • edited Loading

luoyuxia commented Dec 8, 2023

MartijnVisser commented Dec 8, 2023

snuyanzin commented Dec 8, 2023 • edited Loading

snuyanzin commented Dec 8, 2023

snuyanzin commented Dec 8, 2023

boring-cyborg bot commented Dec 8, 2023

snuyanzin Nov 16, 2023 •

edited

Loading

snuyanzin commented Nov 25, 2023 •

edited

Loading

snuyanzin commented Dec 3, 2023 •

edited

Loading

luoyuxia left a comment •

edited

Loading

snuyanzin commented Dec 6, 2023 •

edited

Loading

snuyanzin commented Dec 7, 2023 •

edited

Loading

snuyanzin commented Dec 7, 2023 •

edited

Loading

snuyanzin commented Dec 8, 2023 •

edited

Loading