You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mahout.apache.org by "400Ping (via GitHub)" <gi...@apache.org> on 2026/03/12 07:39:08 UTC

[PR] [QDP] Add GCS remote URL support [mahout]

400Ping opened a new pull request, #1174:
URL: https://github.com/apache/mahout/pull/1174

   ### Related Issues
   
   Closes #1162 
   
   ### Changes
   
   - [ ] Bug fix
   - [x] New feature
   - [ ] Refactoring
   - [x] Documentation
   - [x] Test
   - [ ] CI/CD pipeline
   - [ ] Other
   
   ## Summary
   
   This PR extends QDP remote object storage support from S3-only to S3 + GCS, and clarifies remote URL semantics to avoid ambiguous behavior.
   
   ### What this PR changes
   
   1. Add GCS remote URL support in `qdp-core`:
      - Support `gs://bucket/key` in remote path detection.
      - Build GCS object store client via `GoogleCloudStorageBuilder::from_env`.
      - Enable `object_store` `gcp` feature in `qdp-core/Cargo.toml`.
   
   2. Clarify remote URL parsing behavior:
      - Reject remote URLs containing query/fragment (`?`, `#`) in core parsing.
      - Add matching validation in Python loader builder for fail-fast errors.
      - Keep accepted form explicit: `scheme://bucket/key`.
   
   3. Update tests:
      - Add/extend Rust tests for `gs://` parsing and query/fragment rejection.
      - Update Python loader tests:
        - accept `s3://` and `gs://` remote paths
        - reject query/fragment remote URLs
        - keep streaming `.parquet` validation
   
   4. Update docs:
      - Add `s3://` / `gs://` remote usage examples.
      - Document current limitation: query/fragment is not supported.
      - Sync docs in both `docs/qdp/*` and `website/versioned_docs/version-0.5/qdp/*`.
   
   ## Checklist
   
   - [x] Added or updated unit tests for all changes
   - [x] Added or updated documentation for all 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: commits-unsubscribe@mahout.apache.org

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


Re: [PR] [QDP] Add GCS remote URL support [mahout]

Posted by "400Ping (via GitHub)" <gi...@apache.org>.
400Ping commented on code in PR #1174:
URL: https://github.com/apache/mahout/pull/1174#discussion_r2923291289


##########
qdp/qdp-python/qumat_qdp/loader.py:
##########
@@ -166,10 +166,15 @@ def source_file(self, path: str, streaming: bool = False) -> QuantumDataLoader:
 
         For streaming=True (Phase 2b), only .parquet is supported; data is read in chunks to reduce memory.
         For streaming=False, supports .parquet, .arrow, .feather, .ipc, .npy, .pt, .pth, .pb.
-        Remote paths (s3://) are supported when the remote-io feature is enabled.
+        Remote paths (s3://, gs://) are supported when the remote-io feature is enabled.
+        Remote URL query/fragment (for example ?versionId=... or #...) is not supported.
         """
         if not path or not isinstance(path, str):
             raise ValueError(f"path must be a non-empty string, got {path!r}")
+        if "://" in path and ("?" in path or "#" in path):
+            raise ValueError(
+                "Remote URL query/fragment is not supported; use plain scheme://bucket/key path."
+            )
         # For remote URLs, extract the key portion for extension checks.
         check_path = path.split("?")[0].rsplit("/", 1)[-1] if "://" in path else path

Review Comment:
   For reference: https://github.com/ray-project/ray/pull/61376
   
   This is an actual problem in ray data.



-- 
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: commits-unsubscribe@mahout.apache.org

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


Re: [PR] [QDP] Add GCS remote URL support [mahout]

Posted by "400Ping (via GitHub)" <gi...@apache.org>.
400Ping commented on code in PR #1174:
URL: https://github.com/apache/mahout/pull/1174#discussion_r2923291289


##########
qdp/qdp-python/qumat_qdp/loader.py:
##########
@@ -166,10 +166,15 @@ def source_file(self, path: str, streaming: bool = False) -> QuantumDataLoader:
 
         For streaming=True (Phase 2b), only .parquet is supported; data is read in chunks to reduce memory.
         For streaming=False, supports .parquet, .arrow, .feather, .ipc, .npy, .pt, .pth, .pb.
-        Remote paths (s3://) are supported when the remote-io feature is enabled.
+        Remote paths (s3://, gs://) are supported when the remote-io feature is enabled.
+        Remote URL query/fragment (for example ?versionId=... or #...) is not supported.
         """
         if not path or not isinstance(path, str):
             raise ValueError(f"path must be a non-empty string, got {path!r}")
+        if "://" in path and ("?" in path or "#" in path):
+            raise ValueError(
+                "Remote URL query/fragment is not supported; use plain scheme://bucket/key path."
+            )
         # For remote URLs, extract the key portion for extension checks.
         check_path = path.split("?")[0].rsplit("/", 1)[-1] if "://" in path else path

Review Comment:
   For reference: https://github.com/ray-project/ray/pull/61376
   This is an actual problem happened in ray data.



-- 
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: commits-unsubscribe@mahout.apache.org

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


Re: [PR] [QDP] Add GCS remote URL support [mahout]

Posted by "rich7420 (via GitHub)" <gi...@apache.org>.
rich7420 commented on code in PR #1174:
URL: https://github.com/apache/mahout/pull/1174#discussion_r2923245221


##########
qdp/qdp-python/qumat_qdp/loader.py:
##########
@@ -166,10 +166,15 @@ def source_file(self, path: str, streaming: bool = False) -> QuantumDataLoader:
 
         For streaming=True (Phase 2b), only .parquet is supported; data is read in chunks to reduce memory.
         For streaming=False, supports .parquet, .arrow, .feather, .ipc, .npy, .pt, .pth, .pb.
-        Remote paths (s3://) are supported when the remote-io feature is enabled.
+        Remote paths (s3://, gs://) are supported when the remote-io feature is enabled.
+        Remote URL query/fragment (for example ?versionId=... or #...) is not supported.
         """
         if not path or not isinstance(path, str):
             raise ValueError(f"path must be a non-empty string, got {path!r}")
+        if "://" in path and ("?" in path or "#" in path):
+            raise ValueError(
+                "Remote URL query/fragment is not supported; use plain scheme://bucket/key path."
+            )
         # For remote URLs, extract the key portion for extension checks.
         check_path = path.split("?")[0].rsplit("/", 1)[-1] if "://" in path else path

Review Comment:
   nit: Since query/fragment is already rejected on L174, the `path.split("?")[0]` here is effectively dead code for remote URLs. Could simplify to just `path.rsplit("/", 1)[-1]`, or drop a quick comment noting it's kept as a defensive fallback. Not a big deal either way



-- 
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: commits-unsubscribe@mahout.apache.org

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


Re: [PR] [QDP] Add GCS remote URL support [mahout]

Posted by "rich7420 (via GitHub)" <gi...@apache.org>.
rich7420 commented on code in PR #1174:
URL: https://github.com/apache/mahout/pull/1174#discussion_r2923245221


##########
qdp/qdp-python/qumat_qdp/loader.py:
##########
@@ -166,10 +166,15 @@ def source_file(self, path: str, streaming: bool = False) -> QuantumDataLoader:
 
         For streaming=True (Phase 2b), only .parquet is supported; data is read in chunks to reduce memory.
         For streaming=False, supports .parquet, .arrow, .feather, .ipc, .npy, .pt, .pth, .pb.
-        Remote paths (s3://) are supported when the remote-io feature is enabled.
+        Remote paths (s3://, gs://) are supported when the remote-io feature is enabled.
+        Remote URL query/fragment (for example ?versionId=... or #...) is not supported.
         """
         if not path or not isinstance(path, str):
             raise ValueError(f"path must be a non-empty string, got {path!r}")
+        if "://" in path and ("?" in path or "#" in path):
+            raise ValueError(
+                "Remote URL query/fragment is not supported; use plain scheme://bucket/key path."
+            )
         # For remote URLs, extract the key portion for extension checks.
         check_path = path.split("?")[0].rsplit("/", 1)[-1] if "://" in path else path

Review Comment:
   nit: Since query/fragment is already rejected on L174, the `path.split("?")[0]` here is effectively dead code for remote URLs. Could simplify to just `path.rsplit("/", 1)[-1]`, or drop a quick comment noting it's kept as a defensive fallback. Not a big deal either way 👍



-- 
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: commits-unsubscribe@mahout.apache.org

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


Re: [PR] [QDP] Add GCS remote URL support [mahout]

Posted by "400Ping (via GitHub)" <gi...@apache.org>.
400Ping commented on code in PR #1174:
URL: https://github.com/apache/mahout/pull/1174#discussion_r2923274366


##########
qdp/qdp-python/qumat_qdp/loader.py:
##########
@@ -166,10 +166,15 @@ def source_file(self, path: str, streaming: bool = False) -> QuantumDataLoader:
 
         For streaming=True (Phase 2b), only .parquet is supported; data is read in chunks to reduce memory.
         For streaming=False, supports .parquet, .arrow, .feather, .ipc, .npy, .pt, .pth, .pb.
-        Remote paths (s3://) are supported when the remote-io feature is enabled.
+        Remote paths (s3://, gs://) are supported when the remote-io feature is enabled.
+        Remote URL query/fragment (for example ?versionId=... or #...) is not supported.
         """
         if not path or not isinstance(path, str):
             raise ValueError(f"path must be a non-empty string, got {path!r}")
+        if "://" in path and ("?" in path or "#" in path):
+            raise ValueError(
+                "Remote URL query/fragment is not supported; use plain scheme://bucket/key path."
+            )
         # For remote URLs, extract the key portion for extension checks.
         check_path = path.split("?")[0].rsplit("/", 1)[-1] if "://" in path else path

Review Comment:
   This part is a temporary thing, I plan to add https://github.com/apache/mahout/issues/1173 so I can add this back



-- 
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: commits-unsubscribe@mahout.apache.org

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


Re: [PR] [QDP] Add GCS remote URL support [mahout]

Posted by "400Ping (via GitHub)" <gi...@apache.org>.
400Ping merged PR #1174:
URL: https://github.com/apache/mahout/pull/1174


-- 
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: commits-unsubscribe@mahout.apache.org

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


Re: [PR] [QDP] Add GCS remote URL support [mahout]

Posted by "rich7420 (via GitHub)" <gi...@apache.org>.
rich7420 commented on PR #1174:
URL: https://github.com/apache/mahout/pull/1174#issuecomment-4045140167

   but we still need to fix conflict before merging


-- 
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: commits-unsubscribe@mahout.apache.org

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