Skip to content

[MINOR] fix the error message for glue sync tool#11430

Closed
prabodh1194 wants to merge 1 commit intoapache:masterfrom
prabodh1194:update_glue_sync_tool_err_msg
Closed

[MINOR] fix the error message for glue sync tool#11430
prabodh1194 wants to merge 1 commit intoapache:masterfrom
prabodh1194:update_glue_sync_tool_err_msg

Conversation

@prabodh1194
Copy link
Contributor

@prabodh1194 prabodh1194 commented Jun 11, 2024

Change Logs

When I tried using GLUE mode for syncing meta, I kept getting an error about entering an incorrect mode. I realised a but later that my --sync-tool-classes setting was not pointing to org.apache.hudi.aws.sync.AwsGlueCatalogSyncTool. A better error message will go a long way in improve developer experience in this regard.

Impact

Better error message. No public impact.

Risk level (write none, low medium or high below)

none.

Documentation Update

"none"

Contributor's checklist

  • Read through contributor's guide
  • Change Logs and Impact were stated clearly
  • Adequate tests were added if applicable
  • CI passed

@github-actions github-actions bot added the size:XS PR with lines of changes in <= 10 label Jun 11, 2024
@yihua
Copy link
Contributor

yihua commented Jun 23, 2024

@prabodh1194 is this PR still in draft?

@prabodh1194 prabodh1194 force-pushed the update_glue_sync_tool_err_msg branch from 5b0d785 to 8f2828f Compare June 25, 2024 06:35
@prabodh1194 prabodh1194 marked this pull request as ready for review June 25, 2024 06:35
@prabodh1194
Copy link
Contributor Author

@prabodh1194 is this PR still in draft?

nope. i just wanted to add a test case for this. couldn't do it yet.

@prabodh1194 prabodh1194 force-pushed the update_glue_sync_tool_err_msg branch 3 times, most recently from f269708 to 68f6048 Compare June 29, 2024 16:59
@prabodh1194
Copy link
Contributor Author

@yihua can you please review this now. i finally figured out how to add the correct test case 😄

@github-actions github-actions bot added size:S PR with lines of changes in (10, 100] and removed size:XS PR with lines of changes in <= 10 labels Jun 29, 2024
@prabodh1194 prabodh1194 changed the title fix the error message for glue sync tool [MINOR] fix the error message for glue sync tool Jul 1, 2024
@prabodh1194
Copy link
Contributor Author

@danny0405 can you please review this 🙏🏼

ddlExecutor = new JDBCExecutor(config);
break;
case GLUE:
throw new HoodieHiveSyncException("GLUE mode is supported in AwsGlueCatalogSyncTool class only. "
Copy link
Contributor

@danny0405 danny0405 Jul 1, 2024

Choose a reason for hiding this comment

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

Instread of throwing inside the HoodieHiveSyncClient, I'm wondering why we not just pick the right sync client in the caller.

Copy link
Contributor Author

@prabodh1194 prabodh1194 Jul 1, 2024

Choose a reason for hiding this comment

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

agreed. i can make that change :)

@prabodh1194 prabodh1194 force-pushed the update_glue_sync_tool_err_msg branch from 68f6048 to 74e0b63 Compare July 4, 2024 14:12
@hudi-bot
Copy link
Collaborator

hudi-bot commented Jul 4, 2024

CI report:

Bot commands @hudi-bot supports the following commands:
  • @hudi-bot run azure re-run the last Azure build

@prabodh1194
Copy link
Contributor Author

since the other PR - #11543 is merged, this PR is not required anymore.

@prabodh1194 prabodh1194 closed this Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S PR with lines of changes in (10, 100]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants