-
Notifications
You must be signed in to change notification settings - Fork 143
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
Replace timer with ticker in download progress reporter #3548
Replace timer with ticker in download progress reporter #3548
Conversation
Pinging @elastic/elastic-agent (Team:Elastic-Agent) |
@@ -223,7 +227,6 @@ func (e *Downloader) downloadFile(ctx context.Context, artifactName, filename, f | |||
type downloadProgressReporter struct { | |||
log progressLogger | |||
sourceURI string | |||
timeout time.Duration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is unrelated to this PR per se. But I noticed while working on this PR that this field was never being used, so I'm deleting it as part of this PR.
@@ -75,11 +75,11 @@ func TestDownloadBodyError(t *testing.T) { | |||
} | |||
|
|||
func TestDownloadLogProgressWithLength(t *testing.T) { | |||
fileSize := 100 * units.MiB | |||
fileSize := 100 * units.MB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just to make the math easier.
🌐 Coverage report
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
64d234a
to
adb0a13
Compare
SonarQube Quality Gate |
* Replace timer with ticker in download progress reporter * Adding CHANGELOG entry * Refactoring: extracting common code into helper function (cherry picked from commit 1c5010f)
* Replace timer with ticker in download progress reporter * Adding CHANGELOG entry * Refactoring: extracting common code into helper function (cherry picked from commit 1c5010f) Co-authored-by: Shaunak Kashyap <[email protected]>
What does this PR do?
This PR fixes a bug in the progress logger being used while downloading the Agent artifact during the upgrade process. Prior to this PR the progress logger used a
time.Timer
to log progress of the download. With this PR, the progress logger uses atime.Ticker
instead.Why is it important?
The progress logger should periodically log progress of the download, not just once (after a delay).
Checklist
I have made corresponding changes to the documentationI have made corresponding change to the default configuration files./changelog/fragments
using the changelog toolI have added an integration test or an E2E testRelated issues