fix: remove PR if max attempt reached #430
Merged
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
Signed-off-by: Mykola Rudyk <[email protected]>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #430 +/- ##
============================================
- Coverage 90.93% 90.81% -0.13%
Complexity 489 489
============================================
Files 47 47
Lines 1743 1731 -12
Branches 209 207 -2
============================================
- Hits 1585 1572 -13
- Misses 94 97 +3
+ Partials 64 62 -2 ☔ View full report in Codecov by Sentry. |
o-kopysov
force-pushed
the
m.rudyk/fix_queue_max_attempts
branch
from
f5e3ecd to
a778ef6
Compare
Signed-off-by: Mykola Rudyk <[email protected]>
m-rudyk
force-pushed
the
m.rudyk/fix_queue_max_attempts
branch
from
a778ef6 to
c9ce786
Compare
o-kopysov
reviewed
Feb 14, 2024
m-rudyk
requested review from
tiokim,
MoonkiHong and
tdrozdovsky
as code owners
m-rudyk
force-pushed
the
m.rudyk/fix_queue_max_attempts
branch
2 times, most recently
from
345fda7 to
d755b30
Compare
o-kopysov
reviewed
Feb 14, 2024
| for (LPVSQueue webhook : webhookConfigList) { | ||
| log.info("PROCESSING WebHook id = " + webhook.getId()); | ||
| if (webhook.getAttempts() > maxAttempts) { | ||
| if (webhook.getAttempts() < maxAttempts) { |
There was a problem hiding this comment.
Why "<"?
This is the processing of the case when webhook.getAttempts() > maxAttempts.
And I think we don't need this code here.
o-kopysov
reviewed
Feb 14, 2024
Comment on lines
152
to
159
| LPVSPullRequest pullRequest = new LPVSPullRequest(); | ||
| pullRequest.setPullRequestUrl(webhook.getPullRequestUrl()); | ||
| pullRequest.setUser(webhook.getUserId()); |
There was a problem hiding this comment.
This webhook processing should be added to exception processing before delete().
m-rudyk
force-pushed
the
m.rudyk/fix_queue_max_attempts
branch
from
d755b30 to
c1b015d
Compare
o-kopysov
reviewed
Feb 15, 2024
| delete(webhookConfig); | ||
| int currentAttempts = webhookConfig.getAttempts(); | ||
| if (currentAttempts < maxAttempts) { | ||
| webhookConfig.setAttempts(currentAttempts++); |
There was a problem hiding this comment.
currentAttempts++ increases value after execution setAttempts.
Please use currentAttempts+1 instead.
m-rudyk
force-pushed
the
m.rudyk/fix_queue_max_attempts
branch
3 times, most recently
from
4887195 to
bb9f086
Compare
o-kopysov
reviewed
Feb 15, 2024
| @@ -258,20 +225,36 @@ public void processWebHook(LPVSQueue webhookConfig) { | |||
| log.debug("Creating comment"); | |||
| gitHubService.commentResults(webhookConfig, files, detectedConflicts, pullRequest); | |||
| log.debug("Results posted on GitHub"); | |||
| delete(webhookConfig); | |||
| } else { | |||
| log.warn("Files are not found. Probably pull request is not exists."); | |||
| gitHubService.commentResults(webhookConfig, null, null, pullRequest); | |||
| delete(webhookConfig); | |||
There was a problem hiding this comment.
This delete is incorrect. Please remove it.
m-rudyk
force-pushed
the
m.rudyk/fix_queue_max_attempts
branch
4 times, most recently
from
f283360 to
bf7fd83
Compare
o-kopysov
reviewed
Feb 15, 2024
o-kopysov
reviewed
Feb 15, 2024
o-kopysov
reviewed
Feb 15, 2024
m-rudyk
force-pushed
the
m.rudyk/fix_queue_max_attempts
branch
2 times, most recently
from
88ec88d to
3da70e6
Compare
Signed-off-by: Mykola Rudyk <[email protected]>
m-rudyk
force-pushed
the
m.rudyk/fix_queue_max_attempts
branch
from
3da70e6 to
97d15c0
Compare
|
I've verified the code on local PC: |
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.
Pull Request
Description
Add increment to Queue processing, when PR processing interrupted, it should be kept in queue until max scanning attempts reached.
Type of change
Please delete options that are not relevant.
Testing
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.
Test Configuration:
Checklist: