You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "cshannon (via GitHub)" <gi...@apache.org> on 2023/06/09 14:05:19 UTC
[GitHub] [accumulo] cshannon opened a new pull request, #3476: Improve javadocs for the TabletFile interface
cshannon opened a new pull request, #3476:
URL: https://github.com/apache/accumulo/pull/3476
This improves the javadocs a bit for the new TabletInterface. I'm not the best at wording/descriptions so if anyone else has any ideas for improving the wording feel free to suggest changes.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] cshannon merged pull request #3476: Improve javadocs for the TabletFile interface
Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon merged PR #3476:
URL: https://github.com/apache/accumulo/pull/3476
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] cshannon commented on a diff in pull request #3476: Improve javadocs for the TabletFile interface
Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on code in PR #3476:
URL: https://github.com/apache/accumulo/pull/3476#discussion_r1224524333
##########
core/src/main/java/org/apache/accumulo/core/metadata/TabletFile.java:
##########
@@ -20,15 +20,25 @@
import org.apache.hadoop.fs.Path;
+/**
+ * An interface that represents different types of file references that are handled by code that
+ * processes tablet files.
+ */
public interface TabletFile {
/**
- * @return The file name of the TabletFile
+ * Returns the fileName of the TabletFile. The value return is the name itself and not the entire
+ * path.
+ *
+ * For example, if the full path for a TabletFile is
+ * 'hdfs://nn1/accumulo/tables/5a/t-0001/F0002.rf', this method returns 'F0002.rf'.
Review Comment:
You are actually right, there's probably not a reason to have getFileName(). The caller can just use tabletFile.getPath().getName().
If I drop that method should I open a new PR or just do it here?
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on a diff in pull request #3476: Improve javadocs for the TabletFile interface
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on code in PR #3476:
URL: https://github.com/apache/accumulo/pull/3476#discussion_r1224477599
##########
core/src/main/java/org/apache/accumulo/core/metadata/TabletFile.java:
##########
@@ -20,15 +20,25 @@
import org.apache.hadoop.fs.Path;
+/**
+ * An interface that represents different types of file references that are handled by code that
+ * processes tablet files.
+ */
public interface TabletFile {
/**
- * @return The file name of the TabletFile
+ * Returns the fileName of the TabletFile. The value return is the name itself and not the entire
+ * path.
+ *
+ * For example, if the full path for a TabletFile is
+ * 'hdfs://nn1/accumulo/tables/5a/t-0001/F0002.rf', this method returns 'F0002.rf'.
Review Comment:
Based on this description, it almost seems like we don't need this method, because it's just the substring of `getPath()` since the last index of `/`, which could be done in the calling code. And, with one method, it becomes a functional interface, too, so callers could easily map a stream to the last path element when they need to. In any case, the minimal change is just to fix the javadoc formatting:
Missing `<p>` for separate paragraph, but in this case, I don't think it needs it. Can just use one paragraph.
```suggestion
* Returns the fileName of the TabletFile. The value return is the name itself and not the entire
* path. For example, if the full path for a TabletFile is
* 'hdfs://nn1/accumulo/tables/5a/t-0001/F0002.rf', this method returns 'F0002.rf'.
```
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on a diff in pull request #3476: Improve javadocs for the TabletFile interface
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on code in PR #3476:
URL: https://github.com/apache/accumulo/pull/3476#discussion_r1224538767
##########
core/src/main/java/org/apache/accumulo/core/metadata/TabletFile.java:
##########
@@ -20,15 +20,26 @@
import org.apache.hadoop.fs.Path;
+/**
+ * An interface that represents different types of file references that are handled by code that
+ * processes tablet files.
+ */
public interface TabletFile {
/**
- * @return The file name of the TabletFile
+ * Returns the fileName of the TabletFile. The value return is the name itself and not the entire
+ * path.
+ *
+ * For example, if the full path for a TabletFile is
+ * 'hdfs://nn1/accumulo/tables/5a/t-0001/F0002.rf', this method returns 'F0002.rf'.
*/
String getFileName();
/**
- * @return The path of the TabletFile
+ * Returns the full path for the TabletFile on the file system. The path may be normalized
+ * depending on the specific implementation. For example, a path in hdfs would be returned as
+ * `hdfs://nn1/accumulo/tables/5a/t-0001/F0002.rf`
Review Comment:
```suggestion
* 'hdfs://nn1/accumulo/tables/5a/t-0001/F0002.rf'
```
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on a diff in pull request #3476: Improve javadocs for the TabletFile interface
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on code in PR #3476:
URL: https://github.com/apache/accumulo/pull/3476#discussion_r1224535317
##########
core/src/main/java/org/apache/accumulo/core/metadata/TabletFile.java:
##########
@@ -20,15 +20,25 @@
import org.apache.hadoop.fs.Path;
+/**
+ * An interface that represents different types of file references that are handled by code that
+ * processes tablet files.
+ */
public interface TabletFile {
/**
- * @return The file name of the TabletFile
+ * Returns the fileName of the TabletFile. The value return is the name itself and not the entire
+ * path.
+ *
+ * For example, if the full path for a TabletFile is
+ * 'hdfs://nn1/accumulo/tables/5a/t-0001/F0002.rf', this method returns 'F0002.rf'.
Review Comment:
I would just fix the javadoc here. If you decide we don't need the other method, it can be done in a separate PR, after some experimentation with it to see what that would look like. I'm not sure if there might be undesirable consequences with that change or not.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] EdColeman commented on a diff in pull request #3476: Improve javadocs for the TabletFile interface
Posted by "EdColeman (via GitHub)" <gi...@apache.org>.
EdColeman commented on code in PR #3476:
URL: https://github.com/apache/accumulo/pull/3476#discussion_r1224419748
##########
core/src/main/java/org/apache/accumulo/core/metadata/TabletFile.java:
##########
@@ -20,15 +20,25 @@
import org.apache.hadoop.fs.Path;
+/**
+ * An interface that represents different types of file references that are handled by code that
+ * processes tablet files.
+ */
public interface TabletFile {
/**
- * @return The file name of the TabletFile
+ * Returns the fileName of the TabletFile. The value return is the name itself and not the entire
+ * path.
+ *
+ * For example, if the full path for a TabletFile is
+ * 'hdfs://nn1/accumulo/tables/5a/t-0001/F0002.rf', this method returns 'F0002.rf'.
*/
String getFileName();
/**
- * @return The path of the TabletFile
+ * Returns the full path for the TabletFile on the file system. The path may be normalized
+ * depending on the specific implementation.
Review Comment:
```suggestion
* depending on the specific implementation. For example, a path in hdfs would be returned as `hdfs://nn1/accumulo/tables/5a/t-0001/F0002.rf`
```
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org