feat(storage): support returning skipped items as UserWarning in download_many_to_path

[chandra-siri]

This PR updates download_many_to_path return type in transfer_manager.py

It now returns List[None|Exception|UserWarning]

• None for successful download
• UserWarning for
  - file skipped because skip_if_exists=True was provided and file exists.
  - Resolve path in either invalid or skips destination_directory (because of ..)
• Exception if error occurred during download.

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! 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 significantly improves the download_many_to_path function by making its behavior more transparent when certain files are not downloaded. Instead of simply omitting skipped items from the results, the function now returns a comprehensive list that includes UserWarning objects for each item that was not downloaded, whether due to path traversal security concerns or the skip_if_exists flag. This change provides callers with clear, actionable feedback on the status of every requested download, enhancing debugging and user experience.

Highlights

• Enhanced Return Value for Skipped Downloads: The download_many_to_path function now returns a list that includes UserWarning objects for items that were skipped, providing explicit feedback instead of silently omitting them.
• Improved Error/Warning Handling: Items skipped due to path traversal issues or skip_if_exists=True (when the file already exists) are now explicitly represented by UserWarning instances in the returned list.
• Updated Function Signature and Internal Logic: The return type annotation for download_many_to_path was updated to reflect the possibility of UserWarning objects. The internal call to download_many now consistently uses skip_if_exists=False as the skipping logic is handled upstream within download_many_to_path.
• Comprehensive Test Coverage: New and updated system and unit tests were added to thoroughly validate the new behavior, covering scenarios with successful downloads, skip_if_exists warnings, and path traversal warnings.

Changelog

• google/cloud/storage/transfer_manager.py
  • Modified download_many_to_path to return UserWarning objects for items skipped due to path traversal or skip_if_exists=True.
  • Updated the return type annotation of download_many_to_path to include UserWarning.
  • Adjusted the internal call to download_many to always use skip_if_exists=False, as the skipping logic is now handled within download_many_to_path.
• tests/system/test_transfer_manager.py
  • Updated test_download_many_to_path_skips_download to assert that a UserWarning is returned for skipped items.
  • Added test_download_many_to_path_mixed_results to verify the function's behavior with a mix of successful downloads, skip_if_exists warnings, and path traversal warnings.
• tests/unit/test_transfer_manager.py
  • Modified test_download_many_to_path to assert that the results match the mocked return value.
  • Added test_download_many_to_path_with_skip_if_exists to specifically test the skip_if_exists functionality and the return of UserWarning.
  • Updated test_download_many_to_path_skips_download to assert that a UserWarning is returned for skipped items.
  • Modified test_download_many_to_path_downloads_within_dest_dir to assert that the results match the mocked return value.

Activity

• The pull request was authored by chandra-siri.
• The PR description clearly states the intent: "This PR updates download_many_to_path in transfer_manager.py to return a list of length len(blob_names) containing UserWarning objects for items skipped due to path traversal or skip_if_exists=True, as requested."

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

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.

Customization

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.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 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.

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.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request correctly updates download_many_to_path to return a list containing UserWarning objects for skipped items, addressing both path traversal issues and cases where skip_if_exists=True. The implementation is solid, with the logic for skipped items now encapsulated within the function, and the call to download_many is appropriately adjusted. The tests are comprehensive, including a new system test for mixed success and skipped scenarios. I have one suggestion to enhance the new system test for better coverage.