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.

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

Handle archive upload general progress bar counter incrementation #1019

Merged
merged 85 commits into from
Sep 15, 2024

Conversation

sverdlov93
Copy link
Contributor

@sverdlov93 sverdlov93 commented Sep 11, 2024

  • All tests passed. If this feature is not already covered by the tests, I added new tests.
  • All static analysis checks passed.
  • This pull request is on the dev branch.
  • I used gofmt for formatting the code before submitting the pull request.

Today, all files uploaded as part of an archive are counted for the total number of tasks (denominator), but the numerator are counted only at the end of the zip upload as 1 task.
So for 60k files it's 1/60000 till the end of the upload.
This PR fixes it.

@sverdlov93 sverdlov93 added the improvement Automatically generated release notes label Sep 11, 2024
@sverdlov93 sverdlov93 added the safe to test Approve running integration tests on a pull request label Sep 11, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Sep 11, 2024
Signed-off-by: Michael Sverdlov <[email protected]>
Signed-off-by: Michael Sverdlov <[email protected]>
Signed-off-by: Michael Sverdlov <[email protected]>
Signed-off-by: Michael Sverdlov <[email protected]>
Signed-off-by: Michael Sverdlov <[email protected]>
Signed-off-by: Michael Sverdlov <[email protected]>
@sverdlov93 sverdlov93 added the safe to test Approve running integration tests on a pull request label Sep 12, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Sep 12, 2024
// saveFilesPathsFunc (optional) is a func that is called for each file that is written into the ZIP, and gets the file's local path as a parameter.
func (us *UploadService) readFilesAsZip(archiveDataReader *content.ContentReader, progressPrefix string, flat, symlink bool,
func (us *UploadService) readFilesAsZip(archiveDataReader *content.ContentReader, isZipDryRun, flat, symlink bool,
Copy link
Contributor

Choose a reason for hiding this comment

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

isZipDryRun --> dryRun

In addition to removing the redundant "zip" section from the argument name, that is unnecessary beucase the function's content is already related to the zip, it's preferred to avoid adding the "is" prefix to boolean variables.

Let's change the argument's description as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eyalbe4 its on purpose. We already have a dryRun expression among JFrog CLI command so I didn't want to confuse with that known flag.
Do you have another idea?

Signed-off-by: Michael Sverdlov <[email protected]>
@sverdlov93 sverdlov93 merged commit 71e6988 into jfrog:dev Sep 15, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Automatically generated release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants