Skip to content

Conversation

@SteNicholas
Copy link
Member

When log.scan.timestamp-millis is configured and log.scan isn't configured, the log.scan should be setting to from-timestamp by default.

The brief change log

  • FileStoreTableFactory#validateOptions should validate the case that log.scan.timestamp-millis is configured.

Copy link
Contributor

@JingsongLi JingsongLi left a comment

Choose a reason for hiding this comment

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

Thanks @SteNicholas for the contribution.

Considering that this code is just a verification, there will be some ambiguity in setting the configuration.

I'm mainly worried that this code may be skipped here, and will still be read with full later.

Consider simply returning a StartupInitializer class from options, including scan mode and startup mills. If the mode is null, it returns full if startup mills are configured?

@JingsongLi
Copy link
Contributor

Hi @SteNicholas Can you rebase the latest master? I think we can just set log.scan now, there is a safe validate method now.

@SteNicholas
Copy link
Member Author

@JingsongLi, I have rebased the master branch and addressed above comments. PTAL.

Copy link
Contributor

@JingsongLi JingsongLi left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@JingsongLi JingsongLi merged commit a9b15d6 into apache:master Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants