Apache9 commented on a change in pull request #3952:
URL: https://github.com/apache/hbase/pull/3952#discussion_r771051291
##########
File path:
hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableTracing.java
##########
@@ -100,7 +99,9 @@
private ClientService.Interface stub;
- private AsyncConnection conn;
+ private User user;
Review comment:
Do we really need to declare it as a class member field? It is only used
in one method?
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java
##########
@@ -117,18 +123,29 @@ private boolean isMeta(TableName tableName) {
}
}
- private List<String> getRegionName(RegionLocations locs) {
- List<String> names = new ArrayList<>();
- for (HRegionLocation loc : locs.getRegionLocations()) {
- if (loc != null) {
- names.add(loc.getRegion().getRegionNameAsString());
- }
- }
- return names;
+ private static List<String> getRegionNames(RegionLocations locs) {
+ if (locs == null) { return Collections.emptyList(); }
Review comment:
We still do not have an consensus on the style so let's keep the old
way? At least for me I do not link to have this style of '{}' block, as if the
line is too long, you still need to make a wrapping and then the code will be
like
```
if () {
xxx
}
```
but not
```
if () {
xxx }
```
Seems not consistent...
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/TableOperationSpanBuilder.java
##########
@@ -46,51 +44,61 @@
import org.apache.yetus.audience.InterfaceAudience;
/**
- * Construct {@link io.opentelemetry.api.trace.Span} instances originating from
- * "table operations" -- the verbs in our public API that interact with data
in tables.
+ * Construct {@link Span} instances originating from "table operations" -- the
verbs in our public
+ * API that interact with data in tables.
*/
@InterfaceAudience.Private
-public class TableOperationSpanBuilder implements Supplier<Span> {
+public class TableOperationSpanBuilder<B extends TableOperationSpanBuilder<B>>
+ extends TableSpanBuilder<B> {
// n.b. The results of this class are tested implicitly by way of the likes
of
// `TestAsyncTableTracing` and friends.
private static final String unknown = "UNKNOWN";
- private TableName tableName;
- private final Map<AttributeKey<?>, Object> attributes = new HashMap<>();
+ public TableOperationSpanBuilder(final AsyncConnectionImpl conn) {
+ super(conn);
+ }
+
+ @Override
+ @SuppressWarnings("unchecked")
+ public B self() {
Review comment:
Why we need to have so complicated class hierarchy? A class has a
generic type parameter which is just itself?
--
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]
