Skip to content

Tarball migration utility #3563

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.

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

Merged
merged 3 commits into from
Oct 20, 2023

Conversation

dbutenhof
Copy link
Member

PBENCH-1279

A utility to bulk-migrate Pbench results tarballs to a Pbench Server using the PUT API. This supports checkpointing to more efficiently retry after a failure, and filtering by file system modification date.

@dbutenhof dbutenhof added Server Contrib Operations Related to operation and monitoring of a service labels Oct 20, 2023
@dbutenhof dbutenhof requested a review from webbnh October 20, 2023 12:43
@dbutenhof dbutenhof self-assigned this Oct 20, 2023
webbnh
webbnh previously approved these changes Oct 20, 2023
Copy link
Member

@webbnh webbnh left a comment

Choose a reason for hiding this comment

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

Looks generally excellent -- I didn't find anything I would hold the merge for (although I do have a couple of concerns), but I have a bunch of thoughts to share.

My concerns are the unguarded use of split("::") (which looks like it could cause an error) and the writing of the checkpoint file during preview (which looks like it would allow a user to screw himself a little too easily).

PBENCH-1279

A utility to bulk-migrate Pbench results tarballs to a Pbench Server using the
`PUT` API. This supports checkpointing to more efficiently retry after a
failure, and filtering by file system modification date.
webbnh
webbnh previously approved these changes Oct 20, 2023
Copy link
Member

@webbnh webbnh left a comment

Choose a reason for hiding this comment

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

Looks good, but I still think you should limit the number of splits. (And, I have a few other comments....)

Copy link
Member

@webbnh webbnh left a comment

Choose a reason for hiding this comment

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

👍

@dbutenhof dbutenhof merged commit de55992 into distributed-system-analysis:main Oct 20, 2023
@dbutenhof dbutenhof deleted the migrate branch October 20, 2023 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Contrib Operations Related to operation and monitoring of a service Server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants