Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

docs: remove threads in transfer manager samples #1029

Merged
merged 3 commits into from
May 4, 2023
Merged

docs: remove threads in transfer manager samples #1029

merged 3 commits into from
May 4, 2023

Conversation

MiaCY
Copy link
Contributor

@MiaCY MiaCY commented May 2, 2023

  • Remove deprecated argument "threads" in transfer_manager samples
  • Use "processes" in transfer_manager samples and set default number to be 8
  • Call "max_workers" when calling transfer_manager functions
  • Refactor transfer_manager sample tests accordingly

Fixes #1028 馃

@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: storage Issues related to the googleapis/python-storage API. samples Issues that are directly related to samples. labels May 2, 2023
@MiaCY MiaCY added the owlbot:run Add this label to trigger the Owlbot post processor. label May 3, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label May 3, 2023
@MiaCY MiaCY marked this pull request as ready for review May 3, 2023 04:21
@MiaCY MiaCY requested review from a team as code owners May 3, 2023 04:21
@MiaCY MiaCY requested review from rsamborski and andrewsg May 3, 2023 04:21
@dandhlee dandhlee changed the title docs: remove threads in transfer manager samples sample: remove threads in transfer manager samples May 3, 2023
@dandhlee
Copy link
Contributor

dandhlee commented May 3, 2023

fyi: docs keyword triggers a new release and will be included in the release notes - if this is intended then please revert my title change, otherwise I don't think it's necessary.

andrewsg
andrewsg requested changes May 3, 2023
View reviewed changes
samples/snippets/storage_transfer_manager_upload_directory.py Outdated
# many threads can slow operations, especially with large files, due to
# contention over the Python GIL.
# threads=4
# The maximum number of worker processes that should be used to handle the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this sample doesn't mention the worker type constants, let's try omitting them.

"The maximum number of processes to use for the operation. The performance impact of this value depends on the use case, but smaller files usually benefit from a higher number of processes. Each additional process occupies some CPU and memory resources until finished."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Omitted worker type in comments related to processes. PTAL.

@MiaCY MiaCY added the owlbot:run Add this label to trigger the Owlbot post processor. label May 3, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label May 3, 2023
andrewsg
andrewsg approved these changes May 4, 2023
View reviewed changes
Copy link
Contributor

@andrewsg andrewsg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@andrewsg
Copy link
Contributor

andrewsg commented May 4, 2023

@dandhlee Thanks Dan. Since the samples are public-facing I think they could stand to be in the release notes. I'll merge with docs: for now and if the release notes look messy I'll edit them at launch.

@andrewsg andrewsg changed the title sample: remove threads in transfer manager samples docs: remove threads in transfer manager samples May 4, 2023
@andrewsg andrewsg added the owlbot:run Add this label to trigger the Owlbot post processor. label May 4, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label May 4, 2023
@andrewsg andrewsg merged commit 30c5146 into googleapis:main May 4, 2023
13 checks passed
@release-please release-please bot mentioned this pull request May 4, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewsg andrewsg andrewsg approved these changes

@dandhlee dandhlee dandhlee approved these changes

@rsamborski rsamborski Awaiting requested review from rsamborski rsamborski is a code owner automatically assigned from googleapis/python-samples-reviewers

Assignees
No one assigned
Labels
api: storage Issues related to the googleapis/python-storage API. samples Issues that are directly related to samples. size: m Pull request size is medium.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

docs: deprecate the use of the "threads" parameter in transfer manager samples
3 participants
@MiaCY @dandhlee @andrewsg