You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "RushabhRatnaparkhi (via GitHub)" <gi...@apache.org> on 2026/02/03 06:50:18 UTC

[PR] Make GCS filesystem lookup lazy to match S3 behavior [beam]

RushabhRatnaparkhi opened a new pull request, #37488:
URL: https://github.com/apache/beam/pull/37488

   Resolves #37445
   
   ### What changes were proposed in this pull request?
   
   `FileSystems.get_filesystem()` previously raised a `ValueError` for `gs://`
   paths when GCP dependencies were not installed, while `s3://` paths returned
   a filesystem object and deferred dependency validation until usage time.
   
   This pull request aligns GCS behavior with S3 by making GCS filesystem lookup
   lazy. `get_filesystem()` now returns a `GCSFileSystem` instance without
   requiring `apache-beam[gcp]`, deferring dependency validation until the
   filesystem is actually used (for example, on `open`, `match`, etc.).
   
   A regression test has been added to ensure that GCS filesystem lookup does not
   require GCP extras and that dependency errors are raised only at usage time.
   
   ------------------------
   
   Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [x] Addresses #37445
    - [ ] Update `CHANGES.md` with noteworthy changes (not required; no user-facing API change)
    - [x] This change is small and does not require an Apache ICLA
   
   ### How was this tested?
   
   - Added a unit test verifying that `FileSystems.get_filesystem("gs://...")`
     succeeds without `apache-beam[gcp]` installed.
   - Manually verified that GCP dependency errors are raised only when invoking
     filesystem operations (e.g., `open`) and not during lookup.
   


-- 
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: github-unsubscribe@beam.apache.org

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


Re: [PR] Make GCS filesystem lookup lazy to match S3 behavior [beam]

Posted by "gemini-code-assist[bot] (via GitHub)" <gi...@apache.org>.
gemini-code-assist[bot] commented on PR #37488:
URL: https://github.com/apache/beam/pull/37488#issuecomment-3839413501

   ## Summary of Changes
   
   Hello @RushabhRatnaparkhi, I'm Gemini Code Assist[^1]! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
   
   This pull request standardizes the dependency loading mechanism for cloud storage filesystems within Apache Beam. By making the GCS filesystem lookup lazy, it ensures that the necessary GCP dependencies are only validated and loaded when GCS operations are actually performed, rather than at the initial filesystem object retrieval. This change improves consistency with the existing S3 filesystem behavior and provides a more robust and user-friendly experience by deferring dependency-related errors to the point of usage.
   
   ### Highlights
   
   * **Lazy GCS Dependency Loading**: The GCS filesystem now defers the import of GCP dependencies (apache-beam[gcp]) until an actual GCS operation is attempted, aligning its behavior with S3.
   * **Consistent Error Handling**: FileSystems.get_filesystem() for gs:// paths will no longer raise a ValueError immediately if GCP dependencies are missing; instead, ImportError will be raised only when GCS functionality is used.
   * **New Regression Test**: A unit test has been added to specifically verify that FileSystems.get_filesystem("gs://...") succeeds without requiring apache-beam[gcp] to be installed.
   
   🧠 **New Feature in Public Preview:** You can now enable **Memory** to help **Gemini Code Assist** learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. **Click [here](https://codeassist.google/code-review/login) to enable Memory in your admin console.**
   
   <details>
   <summary><b>Changelog</b></summary>
   
   * **examples/java/src/main/java/org/apache/beam/examples/cookbook/TriggerExample.java**
       * Modified the BigQueryIO.writeTableRows() configuration to explicitly use withWriteDisposition(BigQueryIO.Write.WriteDisposition.WRITE_APPEND).
   * **sdks/python/apache_beam/io/gcp/gcsfilesystem.py**
       * Implemented a _get_gcsio method to encapsulate the gcsio import, making it lazy and raising an ImportError if apache-beam[gcp] is not present.
       * Converted CHUNK_SIZE into a property that dynamically retrieves its value via _get_gcsio, ensuring lazy initialization.
       * Updated all internal methods (_gcsIO, report_lineage) to utilize the new lazy _get_gcsio helper.
   * **sdks/python/apache_beam/io/gcp/gcsfilesystem_test.py**
       * Introduced GCSFileSystemLazyLoadTest to specifically test the lazy loading behavior of get_filesystem for GCS paths.
       * Refactored existing GCS filesystem tests to mock the _get_gcsio method, adapting them to the new lazy dependency loading architecture.
   </details>
   
   
   
   <details>
   <summary><b>Activity</b></summary>
   
   * The pull request was created by RushabhRatnaparkhi.
   * It addresses issue #37445.
   * A unit test was added to confirm that FileSystems.get_filesystem("gs://...") works without apache-beam[gcp] installed.
   * Manual verification was performed to ensure GCP dependency errors are raised only when GCS operations (like open) are invoked, not during initial lookup.
   </details>
   
   
   <details>
   <summary><b>Using Gemini Code Assist</b></summary>
   <br>
   
   The full guide for Gemini Code Assist can be found on our [documentation page](https://developers.google.com/gemini-code-assist/docs/review-github-code), here are some quick tips.
   
   <b>Invoking Gemini</b>
   
   You can request assistance from Gemini at any point by creating a comment using either `/gemini <command>` or `@gemini-code-assist <command>`. Below is a summary of the supported commands on the current page.
   
   Feature | Command | Description
   --- | --- | ---
   Code Review | `/gemini review` | Performs a code review for the current pull request in its current state.
   Pull Request Summary | `/gemini summary` | Provides a summary of the current pull request in its current state.
   Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments.
   Help | `/gemini help` | Displays a list of available commands.
   
   <b>Customization</b>
   
   To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a `.gemini/` folder in the base of the repository. Detailed instructions can be found [here](https://developers.google.com/gemini-code-assist/docs/customize-gemini-behavior-github).
   
   <b>Limitations & Feedback</b>
   
   Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with :thumbsup: and :thumbsdown: on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up [here](https://google.qualtrics.com/jfe/form/SV_2cyuGuTWsEw84yG).
   
   <b>You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the [Gemini Code Assist IDE Extension](https://cloud.google.com/products/gemini/code-assist).</b>
   </details>
   
   
   
   
   [^1]: Review the [Privacy Notices](https://policies.google.com/privacy), [Generative AI Prohibited Use Policy](https://policies.google.com/terms/generative-ai/use-policy), [Terms of Service](https://policies.google.com/terms), and learn how to configure Gemini Code Assist in GitHub [here](https://developers.google.com/gemini-code-assist/docs/customize-gemini-behavior-github). Gemini can make mistakes, so double check it and [use code with caution](https://support.google.com/legal/answer/13505487).
   


-- 
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: github-unsubscribe@beam.apache.org

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


Re: [PR] Make GCS filesystem lookup lazy to match S3 behavior [beam]

Posted by "MansiSingh17 (via GitHub)" <gi...@apache.org>.
MansiSingh17 commented on PR #37488:
URL: https://github.com/apache/beam/pull/37488#issuecomment-3931634695

   @RushabhRatnaparkhi , hi are you working on this, i also want to contribute here. Please, let me know.


-- 
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: github-unsubscribe@beam.apache.org

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


Re: [PR] Make GCS filesystem lookup lazy to match S3 behavior [beam]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37488:
URL: https://github.com/apache/beam/pull/37488#issuecomment-3946856883

   Assigning reviewers:
   
   R: @claudevdm for label python.
   
   
   Note: If you would like to opt out of this review, comment `assign to next reviewer`.
   
   Available commands:
   - `stop reviewer notifications` - opt out of the automated review tooling
   - `remind me after tests pass` - tag the comment author after tests pass
   - `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
   
   The PR bot will only process comments in the main thread (not review comments).


-- 
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: github-unsubscribe@beam.apache.org

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


Re: [PR] Make GCS filesystem lookup lazy to match S3 behavior [beam]

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #37488:
URL: https://github.com/apache/beam/pull/37488#issuecomment-4360023813

   Closing this PR since it looks stale - please reopen if appropriate


-- 
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: github-unsubscribe@beam.apache.org

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


Re: [PR] Make GCS filesystem lookup lazy to match S3 behavior [beam]

Posted by "RushabhRatnaparkhi (via GitHub)" <gi...@apache.org>.
RushabhRatnaparkhi commented on PR #37488:
URL: https://github.com/apache/beam/pull/37488#issuecomment-3945075482

   Please re-run the failing CI checks. The test failure (`test_numeric_strings_preserved` in `textio_test.py`) is a known flaky test related to `OffsetRestrictionTracker` in the dataframe IO module (see [#23948](https://github.com/apache/beam/issues/23948)). This is completely unrelated to the GCS filesystem lazy loading changes in this PR.


-- 
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: github-unsubscribe@beam.apache.org

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


Re: [PR] Make GCS filesystem lookup lazy to match S3 behavior [beam]

Posted by "MansiSingh17 (via GitHub)" <gi...@apache.org>.
MansiSingh17 commented on PR #37488:
URL: https://github.com/apache/beam/pull/37488#issuecomment-3947181264

   Hi @shunping,  I noticed that PR #37488 by @RushabhRatnaparkhi is also actively addressing the same issue and appears to be further along in review. Happy to close this PR if #37488 is preferred, or collaborate if there's value in having both reviewed. Please let me know how you'd like to proceed.


-- 
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: github-unsubscribe@beam.apache.org

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


Re: [PR] Make GCS filesystem lookup lazy to match S3 behavior [beam]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37488:
URL: https://github.com/apache/beam/pull/37488#issuecomment-4031010032

   Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment `assign to next reviewer`:
   
   R: @tvalentyn for label python.
   
   Available commands:
   - `stop reviewer notifications` - opt out of the automated review tooling
   - `remind me after tests pass` - tag the comment author after tests pass
   - `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)


-- 
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: github-unsubscribe@beam.apache.org

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


Re: [PR] Make GCS filesystem lookup lazy to match S3 behavior [beam]

Posted by "MansiSingh17 (via GitHub)" <gi...@apache.org>.
MansiSingh17 commented on PR #37488:
URL: https://github.com/apache/beam/pull/37488#issuecomment-3949311685

   Hi @shunping, thank you for the feedback! Happy to collaborate with Rushabh. @RushabhRatnaparkhi i, would you like to connect? We can combine the cleaner approach from your PR (without the _get_gcsio() helper) with the test fixes from mine. Feel free to reach out and we can coordinate.


-- 
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: github-unsubscribe@beam.apache.org

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


Re: [PR] Make GCS filesystem lookup lazy to match S3 behavior [beam]

Posted by "hjtran (via GitHub)" <gi...@apache.org>.
hjtran commented on PR #37488:
URL: https://github.com/apache/beam/pull/37488#issuecomment-3929487755

   [RushabhRatnaparkhi](https://github.com/RushabhRatnaparkhi) are you still working on this?


-- 
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: github-unsubscribe@beam.apache.org

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


Re: [PR] Make GCS filesystem lookup lazy to match S3 behavior [beam]

Posted by "RushabhRatnaparkhi (via GitHub)" <gi...@apache.org>.
RushabhRatnaparkhi commented on PR #37488:
URL: https://github.com/apache/beam/pull/37488#issuecomment-3938772222

   @hjtran @shunping i am working on this issue and wont mind   collaborating with other contributors


-- 
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: github-unsubscribe@beam.apache.org

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


Re: [PR] Make GCS filesystem lookup lazy to match S3 behavior [beam]

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm closed pull request #37488: Make GCS filesystem lookup lazy to match S3 behavior 
URL: https://github.com/apache/beam/pull/37488


-- 
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: github-unsubscribe@beam.apache.org

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


Re: [PR] Make GCS filesystem lookup lazy to match S3 behavior [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on PR #37488:
URL: https://github.com/apache/beam/pull/37488#issuecomment-3938797025

   Of course! Could you help to review that?


-- 
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: github-unsubscribe@beam.apache.org

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


Re: [PR] Make GCS filesystem lookup lazy to match S3 behavior [beam]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37488:
URL: https://github.com/apache/beam/pull/37488#issuecomment-4011443329

   Reminder, please take a look at this pr: @claudevdm 


-- 
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: github-unsubscribe@beam.apache.org

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


Re: [PR] Make GCS filesystem lookup lazy to match S3 behavior [beam]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37488:
URL: https://github.com/apache/beam/pull/37488#issuecomment-4074614525

   Reminder, please take a look at this pr: @tvalentyn 


-- 
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: github-unsubscribe@beam.apache.org

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


Re: [PR] Make GCS filesystem lookup lazy to match S3 behavior [beam]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #37488:
URL: https://github.com/apache/beam/pull/37488#discussion_r2762803363


##########
sdks/python/apache_beam/io/gcp/gcsfilesystem.py:
##########
@@ -34,16 +34,30 @@
 from apache_beam.io.filesystem import CompressionTypes
 from apache_beam.io.filesystem import FileMetadata
 from apache_beam.io.filesystem import FileSystem
-from apache_beam.io.gcp import gcsio
 
 __all__ = ['GCSFileSystem']
 
 
 class GCSFileSystem(FileSystem):
   """A GCS ``FileSystem`` implementation for accessing files on GCS.
   """
+  def _get_gcsio(self):
+    try:
+      from apache_beam.io.gcp import gcsio
+      return gcsio
+    except ImportError:
+      raise ImportError(
+          'GCSFileSystem requires apache-beam[gcp]. '
+          'Install it with: pip install apache-beam[gcp]')
+
+  _CHUNK_SIZE = None
+
+  @property
+  def CHUNK_SIZE(self):
+    if self._CHUNK_SIZE is None:
+      self._CHUNK_SIZE = self._get_gcsio().MAX_BATCH_OPERATION_SIZE
+    return self._CHUNK_SIZE  # Chuck size in batch operations

Review Comment:
   Typo in comment: "Chuck size" should be "Chunk size".
   ```suggestion
       return self._CHUNK_SIZE  # Chunk size in batch operations
   ```



##########
sdks/python/apache_beam/io/gcp/gcsfilesystem.py:
##########
@@ -34,16 +34,30 @@
 from apache_beam.io.filesystem import CompressionTypes
 from apache_beam.io.filesystem import FileMetadata
 from apache_beam.io.filesystem import FileSystem
-from apache_beam.io.gcp import gcsio
 
 __all__ = ['GCSFileSystem']
 
 
 class GCSFileSystem(FileSystem):
   """A GCS ``FileSystem`` implementation for accessing files on GCS.
   """
+  def _get_gcsio(self):
+    try:
+      from apache_beam.io.gcp import gcsio
+      return gcsio
+    except ImportError:
+      raise ImportError(
+          'GCSFileSystem requires apache-beam[gcp]. '
+          'Install it with: pip install apache-beam[gcp]')
+
+  _CHUNK_SIZE = None
+
+  @property
+  def CHUNK_SIZE(self):
+    if self._CHUNK_SIZE is None:
+      self._CHUNK_SIZE = self._get_gcsio().MAX_BATCH_OPERATION_SIZE
+    return self._CHUNK_SIZE  # Chuck size in batch operations

Review Comment:
   `_CHUNK_SIZE` is defined as a class variable (shared across all instances), but it's being used as an instance variable in the property getter. This creates a subtle bug where if one instance initializes `_CHUNK_SIZE`, all other instances will see it as initialized, even if they haven't accessed the gcsio module yet. Consider making this a true instance variable by initializing it in `__init__` as `self._chunk_size = None` and updating the property accordingly.



##########
examples/java/src/main/java/org/apache/beam/examples/cookbook/TriggerExample.java:
##########
@@ -486,7 +486,10 @@ static void runTriggerExample(TrafficFlowOptions options) throws IOException {
           .get(i)
           .apply(
               "BigQuery Write Rows" + i,
-              BigQueryIO.writeTableRows().to(tableRef).withSchema(getSchema()));
+              BigQueryIO.writeTableRows()
+                  .to(tableRef)
+                  .withSchema(getSchema())
+                  .withWriteDisposition(BigQueryIO.Write.WriteDisposition.WRITE_APPEND));

Review Comment:
   This Java file change appears unrelated to the Python GCS filesystem lazy loading feature described in the PR. The change adds a write disposition to a BigQuery write operation in a Java example, which has no connection to making GCS filesystem lookup lazy in Python. This should likely be in a separate PR.



##########
sdks/python/apache_beam/io/gcp/gcsfilesystem_test.py:
##########
@@ -38,6 +39,12 @@
 # pylint: enable=wrong-import-order, wrong-import-position
 
 
+class GCSFileSystemLazyLoadTest(unittest.TestCase):
+  def test_get_filesystem_does_not_require_gcp_extra(self):
+    fs = FileSystems.get_filesystem('gs://test-bucket/path')
+    self.assertEqual(fs.scheme(), 'gs')

Review Comment:
   The test validates that `get_filesystem()` succeeds without GCP extras, but it doesn't verify the second part of the lazy loading behavior: that actual filesystem operations (like `open()`, `match()`, etc.) properly raise an ImportError when GCP dependencies are missing. Consider adding a test that verifies an ImportError is raised when attempting to use the filesystem without GCP dependencies installed, to ensure the lazy loading works as intended.



-- 
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: github-unsubscribe@beam.apache.org

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


Re: [PR] Make GCS filesystem lookup lazy to match S3 behavior [beam]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37488:
URL: https://github.com/apache/beam/pull/37488#issuecomment-3839651602

   Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment `assign set of reviewers`


-- 
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: github-unsubscribe@beam.apache.org

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


Re: [PR] Make GCS filesystem lookup lazy to match S3 behavior [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on PR #37488:
URL: https://github.com/apache/beam/pull/37488#issuecomment-3947262117

   @MansiSingh17 and @RushabhRatnaparkhi:
   I see good things in both of your PR #37674 and #37488, but I also think they both need some more polishing. 
   - On one hand, #37674 introduced a new `_get_gcsio()` and changed the mocking in the tests, which I think it is clumsy and not necessary.
   - On the other hand, #37488 had some test failure. 
   
   It would be great if we can merge the two solutions together. Could you two collaborate?


-- 
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: github-unsubscribe@beam.apache.org

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


Re: [PR] Make GCS filesystem lookup lazy to match S3 behavior [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on PR #37488:
URL: https://github.com/apache/beam/pull/37488#issuecomment-3937109309

   > @RushabhRatnaparkhi , hi are you working on this, i also want to contribute here. Please, let me know.
   
   Given there is no response in almost a day, I think you are free to take this issue [MansiSingh17](https://github.com/MansiSingh17). I added some hints at https://github.com/apache/beam/issues/37445#issuecomment-3937089947 if you need help.


-- 
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: github-unsubscribe@beam.apache.org

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