-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HBASE-28185 Alter table to set TTL using hbase shell failed when ttl … #5494
Conversation
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what is the semantic for setting TTL as 0, does it mean no TTL?
@@ -184,7 +184,12 @@ private static long humanReadableIntervalToSec(final String humanReadableInterva | |||
hours = matcher.group(6); | |||
minutes = matcher.group(8); | |||
seconds = matcher.group(10); | |||
} else { | |||
LOG.warn( | |||
"Given interval value : {} is not a number and not match human readable format, TTL will set to 0.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Given interval value '{}' is not a number and does not match human readable format, TTL will be set to 0"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, we should not explicitly say 'TTL' here? This is a general method for parsing interval...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for reviwing...
this method just used to convert TTL, it is a private method and just return ttl...although it's name means not just for TTL...
maybe we should change this LOG to
Given interval value : {} is not a number and not match human readable format, value will set to 0.
is it right? i will change it...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's correct the grammar...
"Given interval value '{}' is not a number and does not match human readable format, value will be set to 0"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You still missed two words...
"Given interval value '{}' is not a number and does not match human readable format, value will be set to 0"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You still missed two words...
"Given interval value '{}' is not a number and does not match human readable format, value will be set to 0"
sorry, i forget, i pushed again fixed it.
9c1ab80
to
68825f6
Compare
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
68825f6
to
cc00037
Compare
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
Please fix the checkstyle issue? The line is too long? |
…string is not match format
cc00037
to
7a9ac05
Compare
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
you are right. |
…string is not match format (apache#5494) Signed-off-by: Duo Zhang <zhangduo@apache.org> (cherry picked from commit 027a119) (cherry picked from commit e6f269b) Change-Id: I6a34df4c6457daca5666fc7b827dbb910cd6ae9b
…string is not match format