keith-turner commented on code in PR #5340:
URL: https://github.com/apache/accumulo/pull/5340#discussion_r2049317014
##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java:
##########
@@ -558,6 +559,26 @@ private static Optional<TServerInstance>
checkServer(ClientContext context, Stri
.map(address -> new TServerInstance(address,
stat.getEphemeralOwner()));
}
+ public static void validate(TabletMetadata tm) {
+ if (!tm.fetchedCols.contains(ColumnType.FILES) || !tm.sawPrevEndRow) {
Review Comment:
Maybe part of the problem is that the test are using `new
TabletMetadata.Builder();` which is a package private class that is only
visible in the test because its in the same package. Most test code uses
`TabletMetadata.build(KeyExtent extent);` which is public and works very
differently. When using `new TabletMetadata.Builder();` its internal impl
code that expected to be used in conjunction with
`TabletMetadata.convertRow()`. When using `TabletMetadata.build(KeyExtent
extent);` it will call `TabletMetadata.convertRow()` which will call `new
TabletMetadata.Builder();`.
This is very confusing, took a bit to figure out what was going on here.
Most of the time when writing test `TabletMetadata.Builder()` wil not be
visible so there are not two confusing choices. Because of the package this
test is in, will see two different possible ways to do this. Not a change for
this PR, but we should probably rename `TabletMetadata.Builder` to something
like `TabletMetadata.BuilderImpl`.
For the new unit test in this PR it would probably be good to use
`TabletMetadata.build(KeyExtent extent)` instead of `
TabletMetadata.Builder()`. Doing that the code will get more realistic
TabletMetadata objects that have gone through the same process that would
happen when building a TabletMetadata object from Keys and Values.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]