Skip to content

Comments

Refine update generic https transfer#748

Merged
amyminiter merged 13 commits intomainfrom
refine_update_generic_https_transfer
Jan 23, 2026
Merged

Refine update generic https transfer#748
amyminiter merged 13 commits intomainfrom
refine_update_generic_https_transfer

Conversation

@amyminiter
Copy link
Contributor

Incorporate changes from comments from @folded to refine generic_https_transfer:

  • Replacing boolean --use-wget with wget|curl supports scalability
  • Use quote() for destination path to protect URL.

@amyminiter amyminiter requested review from a team as code owners January 22, 2026 21:51
@amyminiter
Copy link
Contributor Author

@EddieLF thoughts on replacing gsutil with gcloud storage while we're back here? According to this, we should be ok.

Considerations (most cp'd from the documentation0

  • Wildcards treated differently. Not an issue here.
  • When handling errors, gcloud storage cp attempts to copy all resources, even if one of the resources is invalid or doesn't exist. gsutil cp might stop the entire operation as soon as it encounters an invalid resource. ✅ / ❓
  • Logging will be formatted differently. Not an issue for me - you might be affected?
  • gcloud storage cp creates any missing local directories that are specified in the destination path. gsutil cp fails if the destination directory doesn't exist. ✅

Copy link

@folded folded left a comment

Choose a reason for hiding this comment

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

LGTM! thanks!

Copy link
Contributor

@EddieLF EddieLF left a comment

Choose a reason for hiding this comment

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

LGTM, I love discovering this match & case syntax added in python 3.10.

Also happy for you to update gsutil to gcloud storage if you're satisfied that it's totally compatible. Thanks

@amyminiter amyminiter merged commit 3c10411 into main Jan 23, 2026
6 of 10 checks passed
@amyminiter amyminiter deleted the refine_update_generic_https_transfer branch January 23, 2026 04:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants