Skip to content

[594] Support continuous conversion with the RunSync tool - #593

Merged
the-other-tim-brown merged 1 commit into
apache:mainfrom
the-other-tim-brown:continuous-mode
Mar 10, 2025
Merged

[594] Support continuous conversion with the RunSync tool#593
the-other-tim-brown merged 1 commit into
apache:mainfrom
the-other-tim-brown:continuous-mode

Conversation

@the-other-tim-brown

@the-other-tim-brown the-other-tim-brown commented Dec 8, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

What is the purpose of the pull request

Addresses #594, Adds a continuous mode for running RunSync at a scheduled interval

Brief change log

  • Add options to the cmd line args for RunSync to specify whether the tool should run in continuous mode
  • Add option for interval at which the tool runs if continuous mode is set

Verify this pull request

  • Added integration test validating that calling sync without any new data does not corrupt the target tables
  • Added test for RunSync in continuous mode that validates commits are updated by the tool in the background

@the-other-tim-brown the-other-tim-brown changed the title add a loop for continuous conversion with the RunSync tool [594] Add a loop for continuous conversion with the RunSync tool Dec 9, 2024
@the-other-tim-brown the-other-tim-brown changed the title [594] Add a loop for continuous conversion with the RunSync tool [594] Support continuous conversion with the RunSync tool Dec 9, 2024
@the-other-tim-brown
the-other-tim-brown marked this pull request as ready for review March 9, 2025 16:27
vinishjail97
vinishjail97 reviewed Mar 10, 2025
View reviewed changes
Comment thread xtable-utilities/src/main/java/org/apache/xtable/utilities/RunSync.java Outdated
0,
intervalInSeconds,
TimeUnit.SECONDS);
while (!Thread.interrupted()) {

@vinishjail97 vinishjail97 Mar 10, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This method clears the interrupted status for the thread, do we need that here ?
Thread.currentThread().isInterrupted(false) doesn't clear the interrupted state after the check.

@the-other-tim-brown the-other-tim-brown Mar 10, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

nice catch, I need to check the current thread which has an instance method which does not clear the state

@vinishjail97

vinishjail97 commented Mar 10, 2025

Copy link
Copy Markdown
Contributor

Can you squash into one commit before merging ? Thanks !

@the-other-tim-brown
the-other-tim-brown merged commit 546edaa into apache:main Mar 10, 2025
@the-other-tim-brown
the-other-tim-brown deleted the continuous-mode branch March 10, 2025 18:28
This was referenced Mar 14, 2025
Open
Closed
@vinishjail97 vinishjail97 mentioned this pull request Apr 7, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vinishjail97 vinishjail97 vinishjail97 approved these changes
@ashvina ashvina Awaiting requested review from ashvina

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@the-other-tim-brown @vinishjail97