sdks/python: enable recursive deletion for GCSFileSystem Paths

[mohamedawnallah]

Description

In this PR, we enable recursive deletion for Google Cloud Storage (GCS) paths, including directories and blobs. The delete method is updated to remove all blobs under a directory (prefix) when deleting GCS directories.

Additionally, we update the delete test to verify the recursive deletion of directories containing multiple files.

Changes include:

• Enhanced delete method to support recursive directory deletion.
• Added tests for recursive deletion of directories and files.

Fixes #27605

Current Behavior

Before	After

---

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

• Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
• Update CHANGES.md with noteworthy changes.
• If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.

[github-actions]

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

[github-actions]

Assigning reviewers. If you would like to opt out of this review, comment assign to next reviewer:

R: @tvalentyn for label python.
R: @shunping for label io.

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

[mohamedawnallah]

Hi @liferoad, It seems the test cases passed now. I would love to receive any feedback on that PR! 🙏

[shunping]

Ack. Thanks for contributing to Beam. I will review the code today.

[mohamedawnallah]

Hi @shunping, I understand you've been busy last week. Any updates on this PR review? 🙏

[shunping]

Hi @shunping, I understand you've been busy last week. Any updates on this PR review? 🙏

Sorry for the late reply, @mohamedawnallah. I left a comment about potential performance impact with the new change.
Since gcsio has been used heavily in Beam, we have to be very careful about any code change to its public api.

[github-actions]

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

[shunping]

Please make the changes and I am happy to review them again when ready.

[codecov]

Codecov Report

Attention: Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.

Project coverage is 57.46%. Comparing base (98122f3) to head (661f822).
Report is 155 commits behind head on master.

Files with missing lines	Patch %	Lines
sdks/python/apache_beam/io/gcp/gcsfilesystem.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #33611      +/-   ##
============================================
+ Coverage     57.45%   57.46%   +0.01%     
  Complexity     1474     1474              
============================================
  Files           980      984       +4     
  Lines        155541   155804     +263     
  Branches       1076     1076              
============================================
+ Hits          89360    89539     +179     
- Misses        63973    64057      +84     
  Partials       2208     2208              

Flag	Coverage Δ
python	81.22% <87.50%> (-0.05%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[shunping]

Thanks. I think some changes are still needed in gcsfilesystem.py.

[shunping]

LGTM! Thanks again!

[mohamedawnallah]

Hi @shunping,

Thanks for your follow-up review! I've addressed your feedback.

This might be unrelated, but I’d like to use this PR as an opportunity to learn more about Beam development practices as I am getting used to it. My interest was sparked by this discussion: #33672 (comment).

When addressing a review and make a follow-up commit to fix the issue, Someone should avoid squashing reviewed and unreviewed commits. After the follow-up review is completed, would it be necessary to squash the follow-up commit (I am thinking about delay in the merging process e.g for the CI to be triggered again), or is it acceptable to leave the follow-up commit as part of the commit history?

[shunping]

Hi @shunping,

Thanks for your follow-up review! I've addressed your feedback.

This might be unrelated, but I’d like to use this PR as an opportunity to learn more about Beam development practices as I am getting used to it. My interest was sparked by this discussion: #33672 (comment).

When addressing a review and make a follow-up commit to fix the issue, Someone should avoid squashing reviewed and unreviewed commits. After the follow-up review is completed, would it be necessary to squash the follow-up commit (I am thinking about delay in the merging process e.g for the CI to be triggered again), or is it acceptable to leave the follow-up commit as part of the commit history?

The root problem here is about modifying an "already pushed" commit. In your previous PR and this one as well, you pushed your code to github and then you did an operation locally to modify your commit history (eg. squashing the commits or simply amending an already pushed commit). You may notice that, when you pushed your code again, git reminded you to do a "forced" push, which is then shown up in github as something like:

mohamedawnallah force-pushed the enableRecursiveDeleteGCS branch from 661f822 to 3491c70

This is not a good practice for code that is already in the reviewing pipeline. Specifically, when you force-push a commit (which changes the original commit sha1), github can no longer associate it with the reviews that was previously submitted to it. As a result, it will be inconvenient for reviewers to go back and forth to compare what you had changed and what were the previous feedback.

Check the following pages for some more reading.

• https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests#about-pull-requests
• https://jvns.ca/blog/2023/11/06/rebasing-what-can-go-wrong-/#force-pushing-makes-code-reviews-harder

[liferoad]

Can we add this to CHANGES.md?

[shunping]

Can we add this to CHANGES.md?

Good point. @mohamedawnallah, could you add a line under "New Features/Improvement" in https://github.com/apache/beam/blob/master/CHANGES.md to mention your change?

The format is like the following

[GCSIO] Added retry logic to each batch method of the GCS IO (Python)

[shunping]

Thank you!