Log number of remaining retry attempts for asset uploads #5280
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
While retries for asset chunks do work correctly, the messages being logged for failures can be a little confusing. By adding the number of remaining retries in a way similar to other log messages, it gets a lot easier to spot what is really going on. And that the "errors" are actually fairly harmless given the context. Once this has proven itself in production, we might want to redesign the event interface a bit and get rid of a few events. There is a bit of redundancy right now in the logging. But i can see how that might help find issues in the upload logic as it stands right now. I've also cleaned up the upload code a little bit to bring it closer to our current standards, without making changes to the API (yet). Progress: https://progress.opensuse.org/issues/132167
Codecov Report
@@ Coverage Diff @@
## master #5280 +/- ##
=======================================
Coverage 98.31% 98.31%
=======================================
Files 389 389
Lines 37184 37195 +11
=======================================
+ Hits 36558 36569 +11
Misses 626 626
|
|
I also want to increase the retry timeout or the number of attempts, but not sure yet which one would be better here. |
kalikiana
approved these changes
Aug 11, 2023
I feel like this may be nice to make configurable. Not necessarily in this branch. I'd leave it to you, though. |
Martchus
approved these changes
Aug 11, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While retries for asset chunks do work correctly, the messages being logged for failures can be a little confusing. By adding the number of remaining retries in a way similar to other log messages, it gets a lot easier to spot what is really going on. And that the "errors" are actually fairly harmless given the context.
Once this has proven itself in production, we might want to redesign the event interface a bit and get rid of a few events. There is a bit of redundancy right now in the logging. But i can see how that might help find issues in the upload logic as it stands right now.
I've also cleaned up the upload code a little bit to bring it closer to our current standards, without making changes to the API (yet).
Progress: https://progress.opensuse.org/issues/132167