-
Notifications
You must be signed in to change notification settings - Fork 8
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
feat: Added retry
option and max_retries
#50
base: main
Are you sure you want to change the base?
feat: Added retry
option and max_retries
#50
Conversation
f7d3eb7
to
747cb1c
Compare
@ashnamehrotra @sozercan Please review this PR |
@SaptarshiSarkar12 thank you for the PR! I think we can simplify this to one argument. If there are a # of retries specified, we can retry otherwise we don't use retry logic? Would you also be able to modify the test to use these arguments to confirm it works as expected? https://github.com/project-copacetic/copa-action/blob/main/.github/workflows/build.yaml#L83 |
Yes, correct 👍.
@ashnamehrotra Okay. I will 😀. I am out of station now. So, I would make the changes once I return back home (probably 14th of October). Is it fine? |
Signed-off-by: Saptarshi Sarkar <[email protected]>
Signed-off-by: Saptarshi Sarkar <[email protected]>
Signed-off-by: Saptarshi Sarkar <[email protected]>
10c2b75
to
ba65fd0
Compare
… would handle it from now onwards Signed-off-by: Saptarshi Sarkar <[email protected]>
@ashnamehrotra Hi Ashna 👋! Nowadays, I am encountering an issue with copa for the below situation. |
@ashnamehrotra From the failed build log, it seems that there have been too many requests error. Unfortunately, it is a bug reported in aquasecurity/trivy-action#389 and https://github.com/orgs/community/discussions/139074#discussioncomment-10808081 where they suggest using AWS image. |
Hi @SaptarshiSarkar12 this is something we will fix soon in project-copacetic/copacetic#802. This way we will be able to ignore the error of no upgradable packages with the |
Signed-off-by: Saptarshi Sarkar <[email protected]>
…cript Signed-off-by: Saptarshi Sarkar <[email protected]>
Signed-off-by: Saptarshi Sarkar <[email protected]>
@ashnamehrotra Hi Ashna 👋! |
@SaptarshiSarkar12 it would only cause errors if there are no upgradable packages, so you would need to catch/ignore that error until we add support for that error via To resolve the CI tests, you will also need to modify the bats test which runs the copa-action to take in the retry argument. |
Okay, so when would that
@ashnamehrotra I have added |
We are currently prioritizing other issues https://github.com/project-copacetic/copacetic/milestone/8, so it will likely be available in the next milestone v0.11.0 release. It looks like build tests are still failing and the retry argument is not propagating. It would also be good to have a default value 0 set if no arg is provided. |
@ashnamehrotra Okay. Thank you for prioritizing that issue for the next release 😄.
How do I fix that? Any clue?
Okay, I'll add it and let's see if that fixes the build failure 🤞. |
Signed-off-by: Saptarshi Sarkar <[email protected]>
Fixes
Fixes #49
Changes Proposed
retry_on_timeout
boolean to toggle retry featuremax_retries
to take the maximum number of retry as input