[590] Add Hudi Glue Catalog Sync Implementation #649
Conversation
xtable-aws/src/main/java/org/apache/xtable/glue/table/HudiGlueCatalogTableBuilder.java
Outdated
Show resolved
Hide resolved
xtable-aws/src/main/java/org/apache/xtable/glue/GlueCatalogPartitionSyncOperations.java
Show resolved
Hide resolved
xtable-aws/src/main/java/org/apache/xtable/glue/GlueCatalogPartitionSyncOperations.java
Outdated
Show resolved
Hide resolved
xtable-aws/src/main/java/org/apache/xtable/glue/GlueCatalogSyncClient.java
Outdated
Show resolved
Hide resolved
|
@vamsikarnika Can you rebase with latest master ? I will merge this tomorrow. |
xtable-aws/src/test/java/org/apache/xtable/glue/table/TestHudiGlueCatalogTableBuilder.java
Outdated
Show resolved
Hide resolved
xtable-aws/src/test/java/org/apache/xtable/glue/TestGlueCatalogSyncClient.java
Outdated
Show resolved
Hide resolved
vamsikarnika
force-pushed
the
hudi_glue_sync_v1
branch
from
532e35c to
e2a3704
Compare
| tableProperties.putAll(sparkTableProperties); | ||
| return tableProperties; | ||
| } | ||
|
|
There was a problem hiding this comment.
Moved this code to HudiCatalogTablePropertiesExtractor since this was common to Glue as well
vamsikarnika
force-pushed
the
hudi_glue_sync_v1
branch
from
14dc47f to
cdecdec
Compare
| if (response.errors().stream() | ||
| .allMatch( | ||
| (error) -> | ||
| "AlreadyExistsException".equals(error.errorDetail().errorCode()))) { |
There was a problem hiding this comment.
AlreadyExistsException - is there a constant or code you can import from the Glue SDK for this?
There was a problem hiding this comment.
No, I couldn't find any error code for this exceptions. Glue SDK is using the name itself as the error code.
| Table table = | ||
| GlueCatalogTableUtils.getTable( | ||
| glueClient, glueCatalogConfig.getCatalogId(), tableIdentifier); | ||
| ; |
There was a problem hiding this comment.
Remove this extra ;
| assertNotNull(table.getParameters()); | ||
| assertFalse(table.getParameters().isEmpty()); | ||
| assertEquals(table.getParameters().get(HUDI_METADATA_CONFIG), "true"); |
There was a problem hiding this comment.
Why remove the assertions in this test?
There was a problem hiding this comment.
Have moved the logic to fetch table properties to HudiCatalogTablePropertiesExtractor class which populate the fields we're asserting here. Have added the tests in TestHudiCatalogTablePropertiesExractor class to validate these.
|
@vamsikarnika Can you squash into a single commit ? |
vamsikarnika
force-pushed
the
hudi_glue_sync_v1
branch
from
b93b1e8 to
bf14dc0
Compare
Important Read
What is the purpose of the pull request
Add Implementation for Hudi Glue Catalog Sync
Brief change log
Verify this pull request
This change added tests and can be verified as follows: