HBASE-26054 Fix hbase-operator-tools build with HBase 2.4.4 #90
Conversation
|
I guess the build wouldn't catch this, but it will ensure no regressions with older releases. This change is compatible with the older versions and 2.4.4. Build with 2.4.4: hbase-operator-tools % mvn clean install -DskipTests -Dhbase.version=2.4.4 |
|
🎊 +1 overall
This message was automatically generated. |
saintstack
left a comment
saintstack
left a comment
There was a problem hiding this comment.
Nits. Interested in what the failures were that these changes fix... maybe paste up into the JIRA?
hbase-hbck2/pom.xml
Outdated
| <dependency> | ||
| <groupId>com.github.stephenc.findbugs</groupId> | ||
| <artifactId>findbugs-annotations</artifactId> | ||
| <version>${findbugs-annotations.version}</version> |
There was a problem hiding this comment.
Do we need this thing? Can we purge the annotation referenced?
There was a problem hiding this comment.
I'm not very familiar with the findbugs annotations. I guess we could change to using Preconditions or something, but that will be a runtime check. If findbugs is doing something at compile time with those annotations, it might not be a comparable switch.
There was a problem hiding this comment.
Looks like it's all CheckForNull annotations to make spotbugs / findbugs do a better job of warning about usage.
Any particular reason you want to get this out of here @saintstack ? we have the stephenc clean room findbugs dependency in lots of places across the project already.
There was a problem hiding this comment.
@busbey just the usual... lets minimize dependencies if we can. Was wondering if the dependency was just because of some silly, minor usage that we could as well do w/o.
There was a problem hiding this comment.
I may be out of the picture, but in this tool, did we enable any findbugs plugin?
There was a problem hiding this comment.
Stephen beat me to it :) I'm not sure if it's setup correctly for hbase-operator-tools (the test report lists spotbugs as 'spotbugs executables are not available').
|
@saintstack b461d58 Adds the CheckForNull. I also see that it's referenced in core HBase (for example in FSUtils.java). |
|
Added build output from building with 2.4.4 to the JIRA |
|
Also, I'm not sure what our procedure is for upgrading the HBase version in operator tools. I left it at the lower version since the code change works for both versions, but I can upgrade too if it makes sense. |
hbase-hbck2/pom.xml
Outdated
| <dependency> | ||
| <groupId>com.github.stephenc.findbugs</groupId> | ||
| <artifactId>findbugs-annotations</artifactId> | ||
| <version>${findbugs-annotations.version}</version> | ||
| <scope>compile</scope> | ||
| </dependency> |
There was a problem hiding this comment.
[nit] indentation is off 2 spaces
There was a problem hiding this comment.
Good call out, fixed in latest update.
|
I tried to build with IMO if we would like to move with |
|
💔 -1 overall
This message was automatically generated. |
|
Thanks @z-york for explaining why the changes needed. Suggest that updating the hbase dependency is another issue? |
|
We are good to ship this I think. @z-york are you also planning to bump HBase version to 2.4.x on follow up Jira? |
|
Sorry, I submitted this as I was on my way out the door on vacation. I'll commit and open another task for upgrading the version. As for actually running findbugs, if we aren't already doing that, I'll leave that to someone who understands it a bit better :) |
Signed-off-by: Michael Stack <[email protected]> Signed-off-by: Viraj Jasani <[email protected]> Signed-off-by: Stephen Wu <[email protected]>
|
🎊 +1 overall
This message was automatically generated. |
6 participants
No description provided.