You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by "shengdoupi (via GitHub)" <gi...@apache.org> on 2023/04/11 17:12:05 UTC
[GitHub] [shardingsphere] shengdoupi opened a new pull request, #25122: improve properties verification of key generate algorithms
shengdoupi opened a new pull request, #25122:
URL: https://github.com/apache/shardingsphere/pull/25122
Fixes #24750 .
Changes proposed in this pull request:
- Improve initial checks of CosIdSnowflakeKeyGenerateAlgorithm.
- Improve initial checks of SnowflakeKeyGenerateAlgorithm.
---
Before committing this PR, I'm sure that I have checked the following options:
- [x] My code follows the [code of conduct](https://shardingsphere.apache.org/community/en/involved/conduct/code/) of this project.
- [x] I have self-reviewed the commit code.
- [ ] I have (or in comment I request) added corresponding labels for the pull request.
- [x] I have passed maven check locally : `./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e`.
- [ ] I have made corresponding changes to the documentation.
- [x] I have added corresponding unit tests for my changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] shengdoupi commented on a diff in pull request #25122: improve properties verification of key generate algorithms
Posted by "shengdoupi (via GitHub)" <gi...@apache.org>.
shengdoupi commented on code in PR #25122:
URL: https://github.com/apache/shardingsphere/pull/25122#discussion_r1163599690
##########
features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/keygen/SnowflakeKeyGenerateAlgorithm.java:
##########
@@ -113,7 +113,9 @@ private int getMaxVibrationOffset(final Properties props) {
}
private int getMaxTolerateTimeDifferenceMilliseconds(final Properties props) {
- return Integer.parseInt(props.getOrDefault(MAX_TOLERATE_TIME_DIFFERENCE_MILLISECONDS_KEY, MAX_TOLERATE_TIME_DIFFERENCE_MILLISECONDS).toString());
+ int result = Integer.parseInt(props.getOrDefault(MAX_TOLERATE_TIME_DIFFERENCE_MILLISECONDS_KEY, MAX_TOLERATE_TIME_DIFFERENCE_MILLISECONDS).toString());
+ ShardingSpherePreconditions.checkState(result > 0, () -> new KeyGenerateAlgorithmInitializationException(getType(), "Illegal max tolerate time difference milliseconds."));
Review Comment:
> `0` should be allowed here
Thanks! It's already fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] shengdoupi commented on pull request #25122: improve properties verification of key generate algorithms
Posted by "shengdoupi (via GitHub)" <gi...@apache.org>.
shengdoupi commented on PR #25122:
URL: https://github.com/apache/shardingsphere/pull/25122#issuecomment-1505518532
> @shengdoupi Please fix code style.
Done. Sorry for making code style errors.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] shengdoupi commented on a diff in pull request #25122: improve properties verification of key generate algorithms
Posted by "shengdoupi (via GitHub)" <gi...@apache.org>.
shengdoupi commented on code in PR #25122:
URL: https://github.com/apache/shardingsphere/pull/25122#discussion_r1163582857
##########
features/sharding/plugin/cosid/src/main/java/org/apache/shardingsphere/sharding/cosid/algorithm/keygen/CosIdSnowflakeKeyGenerateAlgorithm.java:
##########
@@ -73,7 +73,10 @@ private boolean getAsString(final Properties props) {
}
private long getEpoch(final Properties props) {
- return Long.parseLong(props.getProperty(EPOCH_KEY, DEFAULT_EPOCH + ""));
+ long result = Long.parseLong(props.getProperty(EPOCH_KEY, DEFAULT_EPOCH + ""));
+ ShardingSpherePreconditions.checkState(result > 0L,
+ () -> new ShardingPluginException("Key generate algorithm `%s` initialization failed, reason is: %s.", getType(), "Epoch must be positive."));
Review Comment:
> Should use KeyGenerateAlgorithmInitializationException here.
@Pace2Car Thanks! If I use org.apache.shardingsphere.sharding.exception.algorithm.keygen.KeyGenerateAlgorithmInitializationException here, I need to add dependency on module "shardingsphere-sharding-core", which will cause a circular dependency between "shardingsphere-sharding-cosid" and "shardingsphere-sharding-core". May I create a new KeyGenerateAlgorithmInitializationException class in the same package as ShardingPluginException to solve this problem?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] shengdoupi commented on a diff in pull request #25122: improve properties verification of key generate algorithms
Posted by "shengdoupi (via GitHub)" <gi...@apache.org>.
shengdoupi commented on code in PR #25122:
URL: https://github.com/apache/shardingsphere/pull/25122#discussion_r1163582192
##########
features/sharding/plugin/cosid/src/main/java/org/apache/shardingsphere/sharding/cosid/algorithm/keygen/CosIdSnowflakeKeyGenerateAlgorithm.java:
##########
@@ -73,7 +73,10 @@ private boolean getAsString(final Properties props) {
}
private long getEpoch(final Properties props) {
- return Long.parseLong(props.getProperty(EPOCH_KEY, DEFAULT_EPOCH + ""));
+ long result = Long.parseLong(props.getProperty(EPOCH_KEY, DEFAULT_EPOCH + ""));
+ ShardingSpherePreconditions.checkState(result > 0L,
+ () -> new ShardingPluginException("Key generate algorithm `%s` initialization failed, reason is: %s.", getType(), "Epoch must be positive."));
Review Comment:
Thanks! If I use org.apache.shardingsphere.sharding.exception.algorithm.keygen.KeyGenerateAlgorithmInitializationException here, I need to add dependency on module "shardingsphere-sharding-core", which will cause a circular dependency between "shardingsphere-sharding-cosid" and "shardingsphere-sharding-core". May I create a new KeyGenerateAlgorithmInitializationException class in the same package as ShardingPluginException to solve this problem?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] Pace2Car commented on a diff in pull request #25122: improve properties verification of key generate algorithms
Posted by "Pace2Car (via GitHub)" <gi...@apache.org>.
Pace2Car commented on code in PR #25122:
URL: https://github.com/apache/shardingsphere/pull/25122#discussion_r1163533841
##########
features/sharding/plugin/cosid/src/main/java/org/apache/shardingsphere/sharding/cosid/algorithm/keygen/CosIdSnowflakeKeyGenerateAlgorithm.java:
##########
@@ -73,7 +73,10 @@ private boolean getAsString(final Properties props) {
}
private long getEpoch(final Properties props) {
- return Long.parseLong(props.getProperty(EPOCH_KEY, DEFAULT_EPOCH + ""));
+ long result = Long.parseLong(props.getProperty(EPOCH_KEY, DEFAULT_EPOCH + ""));
+ ShardingSpherePreconditions.checkState(result > 0L,
+ () -> new ShardingPluginException("Key generate algorithm `%s` initialization failed, reason is: %s.", getType(), "Epoch must be positive."));
Review Comment:
Should use KeyGenerateAlgorithmInitializationException here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] RaigorJiang commented on a diff in pull request #25122: improve properties verification of key generate algorithms
Posted by "RaigorJiang (via GitHub)" <gi...@apache.org>.
RaigorJiang commented on code in PR #25122:
URL: https://github.com/apache/shardingsphere/pull/25122#discussion_r1164930071
##########
features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/keygen/SnowflakeKeyGenerateAlgorithm.java:
##########
@@ -113,7 +113,9 @@ private int getMaxVibrationOffset(final Properties props) {
}
private int getMaxTolerateTimeDifferenceMilliseconds(final Properties props) {
- return Integer.parseInt(props.getOrDefault(MAX_TOLERATE_TIME_DIFFERENCE_MILLISECONDS_KEY, MAX_TOLERATE_TIME_DIFFERENCE_MILLISECONDS).toString());
+ int result = Integer.parseInt(props.getOrDefault(MAX_TOLERATE_TIME_DIFFERENCE_MILLISECONDS_KEY, MAX_TOLERATE_TIME_DIFFERENCE_MILLISECONDS).toString());
Review Comment:
Hi @shengdoupi
`Integer.parseInt` may throw `NumberFormatException` (when value is not a number), so try catch is needed here and wrapped as `KeyGenerateAlgorithmInitializationException`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] shengdoupi commented on a diff in pull request #25122: improve properties verification of key generate algorithms
Posted by "shengdoupi (via GitHub)" <gi...@apache.org>.
shengdoupi commented on code in PR #25122:
URL: https://github.com/apache/shardingsphere/pull/25122#discussion_r1166437577
##########
features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/keygen/SnowflakeKeyGenerateAlgorithm.java:
##########
@@ -113,7 +113,9 @@ private int getMaxVibrationOffset(final Properties props) {
}
private int getMaxTolerateTimeDifferenceMilliseconds(final Properties props) {
- return Integer.parseInt(props.getOrDefault(MAX_TOLERATE_TIME_DIFFERENCE_MILLISECONDS_KEY, MAX_TOLERATE_TIME_DIFFERENCE_MILLISECONDS).toString());
+ int result = Integer.parseInt(props.getOrDefault(MAX_TOLERATE_TIME_DIFFERENCE_MILLISECONDS_KEY, MAX_TOLERATE_TIME_DIFFERENCE_MILLISECONDS).toString());
Review Comment:
> OK, then we can submit a separate issue later to handle all `NumberFormatException`.
> Are you interested in getting it done?
OK, I will submit a issue for this later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] Pace2Car commented on a diff in pull request #25122: improve properties verification of key generate algorithms
Posted by "Pace2Car (via GitHub)" <gi...@apache.org>.
Pace2Car commented on code in PR #25122:
URL: https://github.com/apache/shardingsphere/pull/25122#discussion_r1163623704
##########
features/sharding/plugin/cosid/src/main/java/org/apache/shardingsphere/sharding/cosid/algorithm/keygen/CosIdSnowflakeKeyGenerateAlgorithm.java:
##########
@@ -73,7 +73,10 @@ private boolean getAsString(final Properties props) {
}
private long getEpoch(final Properties props) {
- return Long.parseLong(props.getProperty(EPOCH_KEY, DEFAULT_EPOCH + ""));
+ long result = Long.parseLong(props.getProperty(EPOCH_KEY, DEFAULT_EPOCH + ""));
+ ShardingSpherePreconditions.checkState(result > 0L,
+ () -> new ShardingPluginException("Key generate algorithm `%s` initialization failed, reason is: %s.", getType(), "Epoch must be positive."));
Review Comment:
@shengdoupi Sorry, I forgot to check the dependencies, so it's fine to keep it as it is.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] Pace2Car commented on pull request #25122: improve properties verification of key generate algorithms
Posted by "Pace2Car (via GitHub)" <gi...@apache.org>.
Pace2Car commented on PR #25122:
URL: https://github.com/apache/shardingsphere/pull/25122#issuecomment-1504704100
@shengdoupi Please fix code style.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] RaigorJiang commented on a diff in pull request #25122: improve properties verification of key generate algorithms
Posted by "RaigorJiang (via GitHub)" <gi...@apache.org>.
RaigorJiang commented on code in PR #25122:
URL: https://github.com/apache/shardingsphere/pull/25122#discussion_r1164930823
##########
features/sharding/plugin/cosid/src/main/java/org/apache/shardingsphere/sharding/cosid/algorithm/keygen/CosIdSnowflakeKeyGenerateAlgorithm.java:
##########
@@ -73,7 +73,10 @@ private boolean getAsString(final Properties props) {
}
private long getEpoch(final Properties props) {
- return Long.parseLong(props.getProperty(EPOCH_KEY, DEFAULT_EPOCH + ""));
+ long result = Long.parseLong(props.getProperty(EPOCH_KEY, DEFAULT_EPOCH + ""));
Review Comment:
Here it could also be a `NumberFormatException`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] shengdoupi commented on a diff in pull request #25122: improve properties verification of key generate algorithms
Posted by "shengdoupi (via GitHub)" <gi...@apache.org>.
shengdoupi commented on code in PR #25122:
URL: https://github.com/apache/shardingsphere/pull/25122#discussion_r1163599952
##########
features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/algorithm/keygen/SnowflakeKeyGenerateAlgorithmTest.java:
##########
@@ -232,4 +232,10 @@ void assertSetMaxTolerateTimeDifferenceMilliseconds() throws ReflectiveOperation
KeyGenerateAlgorithm algorithm = TypedSPILoader.getService(KeyGenerateAlgorithm.class, "SNOWFLAKE", PropertiesBuilder.build(new Property("max-tolerate-time-difference-milliseconds", "1")));
assertThat(((Properties) Plugins.getMemberAccessor().get(algorithm.getClass().getDeclaredField("props"), algorithm)).getProperty("max-tolerate-time-difference-milliseconds"), is("1"));
}
+
Review Comment:
> Please keep the indentation of the blank line consistent with the previous line.
Thanks! It's already fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] shengdoupi commented on a diff in pull request #25122: improve properties verification of key generate algorithms
Posted by "shengdoupi (via GitHub)" <gi...@apache.org>.
shengdoupi commented on code in PR #25122:
URL: https://github.com/apache/shardingsphere/pull/25122#discussion_r1163600522
##########
features/sharding/plugin/cosid/src/test/java/org/apache/shardingsphere/sharding/cosid/algorithm/keygen/CosIdSnowflakeKeyGenerateAlgorithmTest.java:
##########
@@ -152,4 +152,9 @@ void assertGenerateKeyWhenGreaterThen1023() {
assertThrows(IllegalArgumentException.class, () -> algorithm.setInstanceContext(new InstanceContext(new ComputeNodeInstance(mock(InstanceMetaData.class)), new WorkerIdGeneratorFixture(1024),
new ModeConfiguration("Standalone", null), mock(ModeContextManager.class), mock(LockContext.class), eventBusContext)));
}
+
Review Comment:
> as same.
Thanks! It's already fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] shengdoupi commented on a diff in pull request #25122: improve properties verification of key generate algorithms
Posted by "shengdoupi (via GitHub)" <gi...@apache.org>.
shengdoupi commented on code in PR #25122:
URL: https://github.com/apache/shardingsphere/pull/25122#discussion_r1165717814
##########
features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/keygen/SnowflakeKeyGenerateAlgorithm.java:
##########
@@ -113,7 +113,9 @@ private int getMaxVibrationOffset(final Properties props) {
}
private int getMaxTolerateTimeDifferenceMilliseconds(final Properties props) {
- return Integer.parseInt(props.getOrDefault(MAX_TOLERATE_TIME_DIFFERENCE_MILLISECONDS_KEY, MAX_TOLERATE_TIME_DIFFERENCE_MILLISECONDS).toString());
+ int result = Integer.parseInt(props.getOrDefault(MAX_TOLERATE_TIME_DIFFERENCE_MILLISECONDS_KEY, MAX_TOLERATE_TIME_DIFFERENCE_MILLISECONDS).toString());
Review Comment:
> Hi @shengdoupi `Integer.parseInt` may throw `NumberFormatException` (when value is not a number), so try catch is needed here and wrapped as `KeyGenerateAlgorithmInitializationException`
@RaigorJiang Thanks for your CR! `Integer.parseInt` and `Long.parseLong` are used without try catch in many algorithms' initial check, such as ShardingAutoTableAlgorithm and JobRateLimitAlgorithm, so I'm not sure if it necessary to use try catch here. @Pace2Car Hi, thank you very much for your possible advice.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] Pace2Car commented on a diff in pull request #25122: improve properties verification of key generate algorithms
Posted by "Pace2Car (via GitHub)" <gi...@apache.org>.
Pace2Car commented on code in PR #25122:
URL: https://github.com/apache/shardingsphere/pull/25122#discussion_r1163535606
##########
features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/keygen/SnowflakeKeyGenerateAlgorithm.java:
##########
@@ -113,7 +113,9 @@ private int getMaxVibrationOffset(final Properties props) {
}
private int getMaxTolerateTimeDifferenceMilliseconds(final Properties props) {
- return Integer.parseInt(props.getOrDefault(MAX_TOLERATE_TIME_DIFFERENCE_MILLISECONDS_KEY, MAX_TOLERATE_TIME_DIFFERENCE_MILLISECONDS).toString());
+ int result = Integer.parseInt(props.getOrDefault(MAX_TOLERATE_TIME_DIFFERENCE_MILLISECONDS_KEY, MAX_TOLERATE_TIME_DIFFERENCE_MILLISECONDS).toString());
+ ShardingSpherePreconditions.checkState(result > 0, () -> new KeyGenerateAlgorithmInitializationException(getType(), "Illegal max tolerate time difference milliseconds."));
Review Comment:
`0` should be allowed here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] Pace2Car commented on a diff in pull request #25122: improve properties verification of key generate algorithms
Posted by "Pace2Car (via GitHub)" <gi...@apache.org>.
Pace2Car commented on code in PR #25122:
URL: https://github.com/apache/shardingsphere/pull/25122#discussion_r1163536175
##########
features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/algorithm/keygen/SnowflakeKeyGenerateAlgorithmTest.java:
##########
@@ -232,4 +232,10 @@ void assertSetMaxTolerateTimeDifferenceMilliseconds() throws ReflectiveOperation
KeyGenerateAlgorithm algorithm = TypedSPILoader.getService(KeyGenerateAlgorithm.class, "SNOWFLAKE", PropertiesBuilder.build(new Property("max-tolerate-time-difference-milliseconds", "1")));
assertThat(((Properties) Plugins.getMemberAccessor().get(algorithm.getClass().getDeclaredField("props"), algorithm)).getProperty("max-tolerate-time-difference-milliseconds"), is("1"));
}
+
Review Comment:
Please keep the indentation of the blank line consistent with the previous line.
##########
features/sharding/plugin/cosid/src/test/java/org/apache/shardingsphere/sharding/cosid/algorithm/keygen/CosIdSnowflakeKeyGenerateAlgorithmTest.java:
##########
@@ -152,4 +152,9 @@ void assertGenerateKeyWhenGreaterThen1023() {
assertThrows(IllegalArgumentException.class, () -> algorithm.setInstanceContext(new InstanceContext(new ComputeNodeInstance(mock(InstanceMetaData.class)), new WorkerIdGeneratorFixture(1024),
new ModeConfiguration("Standalone", null), mock(ModeContextManager.class), mock(LockContext.class), eventBusContext)));
}
+
Review Comment:
as same.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] shengdoupi commented on a diff in pull request #25122: improve properties verification of key generate algorithms
Posted by "shengdoupi (via GitHub)" <gi...@apache.org>.
shengdoupi commented on code in PR #25122:
URL: https://github.com/apache/shardingsphere/pull/25122#discussion_r1163582599
##########
features/sharding/plugin/cosid/src/main/java/org/apache/shardingsphere/sharding/cosid/algorithm/keygen/CosIdSnowflakeKeyGenerateAlgorithm.java:
##########
@@ -73,7 +73,10 @@ private boolean getAsString(final Properties props) {
}
private long getEpoch(final Properties props) {
- return Long.parseLong(props.getProperty(EPOCH_KEY, DEFAULT_EPOCH + ""));
+ long result = Long.parseLong(props.getProperty(EPOCH_KEY, DEFAULT_EPOCH + ""));
+ ShardingSpherePreconditions.checkState(result > 0L,
+ () -> new ShardingPluginException("Key generate algorithm `%s` initialization failed, reason is: %s.", getType(), "Epoch must be positive."));
Review Comment:
> Should use KeyGenerateAlgorithmInitializationException here.
Thanks! If I use org.apache.shardingsphere.sharding.exception.algorithm.keygen.KeyGenerateAlgorithmInitializationException here, I need to add dependency on module "shardingsphere-sharding-core", which will cause a circular dependency between "shardingsphere-sharding-cosid" and "shardingsphere-sharding-core". May I create a new KeyGenerateAlgorithmInitializationException class in the same package as ShardingPluginException to solve this problem?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] RaigorJiang commented on a diff in pull request #25122: improve properties verification of key generate algorithms
Posted by "RaigorJiang (via GitHub)" <gi...@apache.org>.
RaigorJiang commented on code in PR #25122:
URL: https://github.com/apache/shardingsphere/pull/25122#discussion_r1166162807
##########
features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/keygen/SnowflakeKeyGenerateAlgorithm.java:
##########
@@ -113,7 +113,9 @@ private int getMaxVibrationOffset(final Properties props) {
}
private int getMaxTolerateTimeDifferenceMilliseconds(final Properties props) {
- return Integer.parseInt(props.getOrDefault(MAX_TOLERATE_TIME_DIFFERENCE_MILLISECONDS_KEY, MAX_TOLERATE_TIME_DIFFERENCE_MILLISECONDS).toString());
+ int result = Integer.parseInt(props.getOrDefault(MAX_TOLERATE_TIME_DIFFERENCE_MILLISECONDS_KEY, MAX_TOLERATE_TIME_DIFFERENCE_MILLISECONDS).toString());
Review Comment:
OK, then we can submit a separate issue later to handle all `NumberFormatException`.
Are you interested in getting it done?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] RaigorJiang merged pull request #25122: improve properties verification of key generate algorithms
Posted by "RaigorJiang (via GitHub)" <gi...@apache.org>.
RaigorJiang merged PR #25122:
URL: https://github.com/apache/shardingsphere/pull/25122
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org