-
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-26784 Use HIGH_QOS for ResultScanner.close requests #4163
Conversation
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
The remaining checkstyle warnings are all ImportOrder, which are hard to understand. The imports look properly sorted to me. Looking at our checkstyle.xml, I see there are some groups defined so I figured maybe it was that. But we don't have I tried running mvn checkstyle:check locally, but it failed to run at all. Without that it'd be hard to iterate on this using the CI to check my ordering, so if someone has guidance I'd be happy to make changes. Otherwise I'll just leave that as is. |
@bbeaudreault Import order is a bit counterintuitive. It goes like this:
|
Thanks again! I gave that a shot |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@bbeaudreault if you happen to use an editor that supports EditorConfig, the import order is defined in our |
Thanks @ndimiduk I was not aware of that file. My company has an IntelliJ plugin which overrides may of the defaults to work for our internal requirements, so that's probably why i wasn't able to take advantage of that file by default. I've updated it now and that will be very helpful to avoid all of these import issues :) |
This PR should be all set -- the build shows checks failed, but all 3 post-backs are +1's. No checkstyle/javac issues remaining. |
…quests (apache#4163) Signed-off-by: Duo Zhang <zhangduo@apache.org> Signed-off-by: Andrew Purtell <apurtell@apache.org> Signed-off-by: Xiaolin Ha <haxiaolin@apache.org>
…quests (apache#4163) Signed-off-by: Duo Zhang <zhangduo@apache.org> Signed-off-by: Andrew Purtell <apurtell@apache.org> Signed-off-by: Xiaolin Ha <haxiaolin@apache.org>
Signed-off-by: Duo Zhang <zhangduo@apache.org> Signed-off-by: Andrew Purtell <apurtell@apache.org> Signed-off-by: Xiaolin Ha <haxiaolin@apache.org>
The first commit here is cherry-picked (no conflicts) from the master branch PR: #4146
The second commit adds support for the branch-2 specific blocking client, as well as a test inspired by the async test. While the test classes might look somewhat similar, there are subtle differences that make it hard to DRY. So I just kept it simple with 2 distinct classes.