Send labels to Codecov only after collecting them #191
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
|
We can still make this behavior optional by adding some flag to enable/disable it. |
giovanni-guidini
force-pushed
the
gio/change-label-analysis
branch
from
c679bef
to
8c0eff1
Compare
Codecov Report
@@ Coverage Diff @@
## master #191 +/- ##
==========================================
+ Coverage 95.67% 95.76% +0.08%
==========================================
Files 70 70
Lines 2475 2500 +25
==========================================
+ Hits 2368 2394 +26
+ Misses 107 106 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
dana-yaish
reviewed
Jul 10, 2023
| else: | ||
| # Initial request with no labels failed | ||
| # Retry it | ||
| eid = _send_labelanalysis_request(payload, url, token_header) |
There was a problem hiding this comment.
I can see that we patch labels in the first trial, but we don't do that if the first one failed and we retried?
There was a problem hiding this comment.
Exactly. If the first request failed, then Codecov haven't started the label analysis task yet. So instead of simply patching the labels we need to send that POST request again. Since we already have the labels at this point we can send them in the request, like it was done before.
giovanni-guidini
force-pushed
the
gio/change-label-analysis
branch
from
8c0eff1
to
33921e2
Compare
giovanni-guidini
force-pushed
the
gio/change-label-analysis
branch
from
33921e2
to
9324e83
Compare
These changes alter the way label analysis command works by making 2 requests to Codecov instead of 1. Why would we want to do this, if it's more time to make a second request? Because it takes (usually) longer to collect the labels than it takes to make a second request. Specially for bigger repos, collecting the labels can take dozens of seconds. By making the label collection parallel with the labels processing we can speed up the total ATS time. We still PATCH the labels in case the worker hasn't finished the calculations, and it will correctly refresh the request labels from the DB (based on [here](https://github.com/codecov/worker/pull/1210)). However even if it misses the requested labels we recalculate the `absent_labels`, if needed (`_potentially_calculate_absent_labels`). So we still guarantee the exact same response everytime. Potentially faster. PS.: The file `test_instantiation.py` changes because of the linter
giovanni-guidini
force-pushed
the
gio/change-label-analysis
branch
from
9324e83
to
6b548fe
Compare
dana-yaish
approved these changes
Jul 14, 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.
These changes alter the way label analysis command works by making 2 requests to Codecov instead of 1.
Why would we want to do this, if it's more time to make a second request?
Because it takes (usually) longer to collect the labels than it takes to make a second request.
Specially for bigger repos, collecting the labels can take dozens of seconds.
By making the label collection parallel with the labels processing we can speed up the total ATS time.
We still PATCH the labels in case the worker hasn't finished the calculations, and it will
correctly refresh the request labels from the DB (based on here).
However even if it misses the requested labels we recalculate the
absent_labels, if needed (_potentially_calculate_absent_labels).So we still guarantee the exact same response everytime. Potentially faster.
PS.: The file
test_instantiation.pychanges because of the linter