ctubbsii commented on code in PR #4082:
URL: https://github.com/apache/accumulo/pull/4082#discussion_r1430888133
##########
core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java:
##########
@@ -27,64 +27,106 @@
import org.apache.accumulo.core.data.Value;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.LogColumnFamily;
import org.apache.hadoop.io.Text;
+import org.checkerframework.checker.nullness.qual.NonNull;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.net.HostAndPort;
-public class LogEntry {
+public final class LogEntry {
private final String filePath;
+ private final HostAndPort tserver;
+ private final UUID uniqueId;
- public LogEntry(String filePath) {
- validateFilePath(filePath);
+ private LogEntry(String filePath, HostAndPort tserver, UUID uniqueId) {
this.filePath = filePath;
- }
-
- public String getFilePath() {
- return this.filePath;
+ this.tserver = tserver;
+ this.uniqueId = uniqueId;
}
/**
- * Validates the expected format of the file path. We expect the path to
contain a tserver
- * (host:port) followed by a UUID as the file name. For example,
- * localhost:1234/927ba659-d109-4bce-b0a5-bcbbcb9942a2 is a valid file path.
+ * Creates a new LogEntry object after validating the expected format of the
file path. We expect
+ * the path to contain a tserver (host+port) followed by a UUID as the file
name as the last two
+ * components.<br>
+ * For example,
file:///some/dir/path/localhost+1234/927ba659-d109-4bce-b0a5-bcbbcb9942a2 is a
+ * valid file path.
*
* @param filePath path to validate
+ * @return an object representation of this log entry
* @throws IllegalArgumentException if the filePath is invalid
*/
- private static void validateFilePath(String filePath) {
+ public static LogEntry fromFilePath(String filePath) {
Review Comment:
I thought of that, but I didn't for a few reasons:
1. I'm hesitant to more tightly couple HDFS types in code when I'd like to
eventually get all of it out of our code (as much as possible)
2. There's already some confusion in that the Hadoop Path type is used in
two of the three relevant places in code, but the Java NIO Path type is used in
tests, and toString works for both right now, so I didn't want to make the
changes too big
3. I'm uncertain the impact of using new Path for the HDFS Path type, if
there's any kind of normalization or transformation, and I wanted to do simple
refactorings, without changing any behavior that might risk breakage
--
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]
