fix: aws glue sync set comment #14234

parisni · 2025-11-07T22:44:48Z

Describe the issue this Pull Request addresses

the #9347 PR introduced a regression on the aws glue comments when moving to the aws sdk v2. This restore the original behavior

Summary and Changelog

Impact

Risk Level

Documentation Update

Contributor's checklist

Read through contributor's guide
Enough context is provided in the sections above
Adequate tests were added if applicable

yihua

LGTM. Possible to add a test case on this?

parisni · 2025-11-08T22:36:34Z

@yihua

added base IT test with moto to emulate aws glue + hive test tools to emulate hudi local table
to make glue client connect to moto i did introduce sts configs
tested and fixed the setComment implem
~~dropped~~ improved ~~useless~~ partition pushdown tests
removed moto/dynamodb platform and also use moto official image (i m working on arm64 and the current setup couln't work)

parisni · 2025-11-08T23:30:49Z

 Error:  testRemoveTableComments  Time elapsed: 0.03 s  <<< ERROR!
java.lang.NoSuchMethodError: org.apache.parquet.schema.Types$PrimitiveBuilder.as(Lorg/apache/parquet/schema/LogicalTypeAnnotation;)Lorg/apache/parquet/schema/Types$Builder;
	at org.apache.hudi.aws.sync.ITTestAWSGlueComments.setUp(ITTestAWSGlueComments.java:58)

locally i run the IT with spark profile 3.3 successfuly. The CI run spark 3.5. Sounds like there is a parquet deps conflict on this profile

fix problem with array fix array of struct

parisni · 2025-11-12T12:52:08Z

@yihua I added a test framework that will be useful to validate the aws binding. what about landing this fix into 1.1 ?

parisni · 2025-11-12T19:48:52Z

also maybe @danny0405 ?

danny0405 · 2025-11-13T02:19:22Z

There is a test failure:

[ERROR] Errors: 
[ERROR]   TestAwsGlueSyncTool.validateInitThroughSyncTool:96 » Hoodie Unable to instanti...

danny0405 · 2025-11-13T03:03:30Z

hudi-aws/src/main/java/org/apache/hudi/aws/sync/AWSGlueCatalogSyncClient.java


-  private void setComments(List<Column> columns, Map<String, Option<String>> commentsMap) {
-    columns.forEach(column -> {
+  private List<Column> setComments(List<Column> columns, Map<String, Option<String>> commentsMap) {


The original logic does not take any effect right?

That's the point. BTW the test did prove the original method fail. New implem fixes the tests

Can you fix the test failures.

danny0405 · 2025-11-13T03:09:01Z

hudi-aws/src/test/java/org/apache/hudi/aws/sync/ITTestGluePartitionPushdown.java

-                              )
-                              .build())
-                    .build()).build()).get();
+  public void setUpPushdownTest() throws Exception {


so we can ensure the super class setUp() been executed before this one?

The answer is true if the method is not overidden.

danny0405

+1, let's fix the test failures.

add tests fix deps restore partition pushd down it fix checkstyle fix test

parisni · 2025-11-13T09:05:01Z

@danny0405 i fixed the tests, let wait the CI to confirm. Note that without #14235 the comments still won't be added. The reason is the comments are coming from the tableResolver = the avro schema of the last commit. While hudi stores the comments in the avro, the tableResolver doesn't extract them. That's what #14235 provides. Let me fix the CI there

danny0405 · 2025-11-14T03:57:19Z

hudi-aws/pom.xml

                                </image>
                                <image>
-                                    <name>apachehudi/moto:${moto.version}</name>
+                                    <name>motoserver/moto:${moto.version}</name>


not sure what the exact effect of the change

The hudi image is not cross platform. Only amd64. The motoserver is cross platform. Not sure whonisballowednto push hudi images to dockerhub

danny0405 · 2025-11-14T03:57:30Z

hudi-aws/pom.xml

                                    <name>amazon/dynamodb-local:${dynamodb-local.version}</name>
                                    <alias>it-database</alias>
                                    <run>
-                                        <platform>linux/amd64</platform>


why remove this

danny0405 · 2025-11-14T03:57:36Z

hudi-aws/pom.xml

+                                    <name>motoserver/moto:${moto.version}</name>
                                    <alias>it-aws</alias>
                                    <run>
-                                        <platform>linux/amd64</platform>


why remove this

This makes the image agnostic to the platform. Pining to amd64 did not allow to run the test on arm64 or apple silicon

danny0405 · 2025-11-15T03:01:33Z

@hudi-bot run azure

danny0405 · 2025-11-18T03:54:06Z

@hudi-bot run azure

danny0405 · 2025-11-18T09:03:27Z

@parisni can you retrigger this PR CI by a dummy commit, I can not trigger it through the cmd.

parisni · 2025-11-18T10:10:28Z

@danny0405 done. also in the other PR

danny0405 · 2025-11-20T03:55:38Z

@hudi-bot run azure

yihua

@parisni Could you rebase your branch on top of the latest master? A few fixes on CI flakiness have been merged.

yihua

LGTM overall. @parisni Could you check CI failures and rebase the PR on master?

parisni · 2025-12-22T14:13:18Z

Hi @yihua @danny0405 i did again rebase on master.

hudi-bot · 2025-12-22T15:14:51Z

CI report:

99bb5ae Azure: FAILURE

Bot commands

@hudi-bot supports the following commands:

@hudi-bot run azure re-run the last Azure build

github-actions bot added the size:XS PR with lines of changes in <= 10 label Nov 7, 2025

parisni marked this pull request as draft November 7, 2025 22:46

yihua reviewed Nov 7, 2025

View reviewed changes

github-actions bot added size:L PR with lines of changes in (300, 1000] and removed size:XS PR with lines of changes in <= 10 labels Nov 8, 2025

parisni force-pushed the feat-aws-glue-comment branch from bb2fda9 to abf8e3d Compare November 8, 2025 22:37

parisni changed the title ~~feat: fix aws glue sync set comment~~ fix: aws glue sync set comment Nov 8, 2025

parisni requested a review from yihua November 8, 2025 22:49

parisni marked this pull request as ready for review November 8, 2025 22:49

parisni force-pushed the feat-aws-glue-comment branch 2 times, most recently from 5d5d659 to 7bafc7a Compare November 10, 2025 08:54

parisni added a commit to leboncoin/hudi that referenced this pull request Nov 10, 2025

fix: aws glue sync set comment apache#14234

5f5f385

parisni added a commit to leboncoin/hudi that referenced this pull request Nov 10, 2025

fix: aws glue sync set comment apache#14234

b0b422c

parisni added a commit to leboncoin/hudi that referenced this pull request Nov 10, 2025

fix: aws glue sync set comment apache#14234

d620dbc

fix problem with array fix array of struct

parisni mentioned this pull request Nov 12, 2025

[HUDI-7399] hudi-aws sync integration testing #10614

Closed

4 tasks

danny0405 reviewed Nov 13, 2025

View reviewed changes

danny0405 approved these changes Nov 13, 2025

View reviewed changes

feat: fix aws glue sync set comment

5613bfe

add tests fix deps restore partition pushd down it fix checkstyle fix test

parisni force-pushed the feat-aws-glue-comment branch from 7bafc7a to 5613bfe Compare November 13, 2025 08:58

danny0405 reviewed Nov 14, 2025

View reviewed changes

Dummy commit to trigger the ci

c9b1877

yihua reviewed Nov 28, 2025

View reviewed changes

Merge remote-tracking branch 'origin/master' into feat-aws-glue-comment

0e56ad6

yihua reviewed Dec 13, 2025

View reviewed changes

Merge remote-tracking branch 'origin/master' into feat-aws-glue-comment

99bb5ae

fix: aws glue sync set comment #14234

Are you sure you want to change the base?

fix: aws glue sync set comment #14234

Uh oh!

Conversation

parisni commented Nov 7, 2025

Describe the issue this Pull Request addresses

Summary and Changelog

Impact

Risk Level

Documentation Update

Contributor's checklist

Uh oh!

yihua left a comment

Choose a reason for hiding this comment

Uh oh!

parisni commented Nov 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

parisni commented Nov 8, 2025

Uh oh!

parisni commented Nov 12, 2025

Uh oh!

parisni commented Nov 12, 2025

Uh oh!

danny0405 commented Nov 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danny0405 left a comment

Choose a reason for hiding this comment

Uh oh!

parisni commented Nov 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danny0405 commented Nov 15, 2025

Uh oh!

danny0405 commented Nov 18, 2025

Uh oh!

danny0405 commented Nov 18, 2025

Uh oh!

parisni commented Nov 18, 2025

Uh oh!

danny0405 commented Nov 20, 2025

Uh oh!

yihua left a comment

Choose a reason for hiding this comment

Uh oh!

yihua left a comment

Choose a reason for hiding this comment

Uh oh!

parisni commented Dec 22, 2025

Uh oh!

hudi-bot commented Dec 22, 2025

CI report:

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

parisni commented Nov 8, 2025 •

edited

Loading