udim commented on a change in pull request #12805:
URL: https://github.com/apache/beam/pull/12805#discussion_r490652676



##########
File path: 
sdks/java/extensions/google-cloud-platform-core/src/test/java/org/apache/beam/sdk/extensions/gcp/util/gcsfs/GcsPathTest.java
##########
@@ -175,6 +175,39 @@ public void testGcsPathToString() throws Exception {
     assertEquals(filename, path.toString());
   }
 
+  @Test
+  public void testGcsPath_withGeneration() {
+    String filename = "gs://some_bucket/some/file.txt#12345";

Review comment:
       What happens when the URL ends in `#`?
   Could you add that test case?

##########
File path: 
sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/gcsfs/GcsPath.java
##########
@@ -103,7 +103,7 @@ public static GcsPath fromUri(URI uri) {
    * <p>This is used to separate the components. Verification is handled 
separately.
    */
   public static final Pattern GCS_URI =
-      Pattern.compile("(?<SCHEME>[^:]+)://(?<BUCKET>[^/]+)(/(?<OBJECT>.*))?");

Review comment:
       Any reason why the group names were removed?
   The more groups the easier it they are to track with names, no?

##########
File path: 
sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/gcsfs/GcsPath.java
##########
@@ -228,6 +248,14 @@ public FileSystem getFileSystem() {
     return fs;
   }
 
+  public Long getGeneration() {
+    return generation;
+  }
+
+  public boolean hasGeneration() {
+    return generation != null && generation != -1L;

Review comment:
       When is `generation` expected to be -1?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to