Skip to content

[ez] Use strip for arg sanitization in upload_metadata_file to improve readability #144155

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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

[ez] Use strip for arg sanitization in upload_metadata_file to improve readability #144155

wants to merge 2 commits into from

Conversation

clee2000
Copy link
Contributor

@clee2000 clee2000 commented Jan 3, 2025 •
edited
Loading

Minor thing that improves readability. I didn't realize you could specify characters for strip when I wrote this

@pytorch-bot pytorch-bot bot added the topic: not user facing topic category label Jan 3, 2025
@pytorch-bot PyTorch Bot
Copy link

pytorch-bot bot commented Jan 3, 2025 •
edited
Loading

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/144155

Note: Links to docs will display an error until the docs builds have been completed.

⏳ No Failures, 14 Pending

As of commit 6f7f9b0 with merge base b9fbd65 (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@clee2000 clee2000 marked this pull request as ready for review January 3, 2025 17:21
@clee2000 clee2000 changed the title Use r/lstrip for arg sanitization in upload_metadata_file to improve readability [ez] Use r/lstrip for arg sanitization in upload_metadata_file to improve readability Jan 3, 2025
@clee2000 clee2000 requested a review from a team January 3, 2025 17:21
huydhn
huydhn reviewed Jan 3, 2025
View reviewed changes
scripts/release/upload_metadata_file.py Outdated
args.key_prefix = args.key_prefix[1:]
if args.key_prefix.endswith("/"):
args.key_prefix = args.key_prefix[:-1]
args.bucket = args.bucket.rstrip("/")
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason not to make it even shorter with args.bucket.strip("/")?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because I didn't realize strip could also take in characters... Changed

@clee2000 clee2000 changed the title [ez] Use r/lstrip for arg sanitization in upload_metadata_file to improve readability [ez] Use strip for arg sanitization in upload_metadata_file to improve readability Jan 3, 2025
@clee2000
Copy link
Contributor Author

clee2000 commented Jan 3, 2025

@pytorchbot merge -f "no trunk needed"

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use -f as last resort and instead consider -i/--ignore-current to continue the merge ignoring current failures. This will allow currently pending tests to finish and report signal before the merge.

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@github-actions github-actions bot deleted the csl/upload_metadta_better_parsing branch February 3, 2025 02:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@huydhn huydhn huydhn approved these changes

@Skylion007 Skylion007 Skylion007 approved these changes

Assignees

No one assigned

Labels

Merged topic: not user facing topic category

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@clee2000 @pytorchmergebot @huydhn @Skylion007