ndimiduk commented on a change in pull request #3952:
URL: https://github.com/apache/hbase/pull/3952#discussion_r771633306
##########
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:
Nope, I suppose not.
##########
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:
```java
if () {
xxx }
```
I agree that this should not be allowed style.
Let's keep this PR about the feature, not the style. This is a commit I made
some time ago, before #3913 . I'll undo this style until we have consensus.
##########
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:
I don't like it either. Java really sucks for inheritance with the
builder pattern. I could try to rebuild it using mix-ins, but I think that will
be equally complex.
--
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]
