Skip to content
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

Retry apt-get install for memory-leaks config #3953

Merged
merged 2 commits into from
Sep 26, 2024
Merged

Conversation

p-datadog
Copy link
Contributor

What does this PR do?

This configuration is frequently failing due to HTTP 503 error when installing valgrind. Retry the installation one time after a short break to see if this would make the configuration less flaky.

Motivation:
Repeated test failures

Additional Notes:

How to test the change?

Unsure? Have a question? Request a review!

This configuration is frequently failing due to HTTP 503 error
when installing valgrind. Retry the installation one time
after a short break to see if this would make the configuration
less flaky.
@p-datadog p-datadog requested a review from a team as a code owner September 25, 2024 14:49
Copy link
Member

@ivoanjo ivoanjo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I'm not sure if this will fix it, but it's cheap to try

@@ -11,7 +11,7 @@ jobs:
bundler-cache: true # runs 'bundle install' and caches installed gems automatically
bundler: latest
cache-version: v1 # bump this to invalidate cache
- run: sudo apt update && sudo apt install -y valgrind && valgrind --version
- run: sudo apt-get update && (sudo apt-get install -y valgrind || sleep 5 && sudo apt-get install -y valgrind) && valgrind --version
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: Modern versions of apt don't need apt-get (they're mostly aliases)

Suggested change
- run: sudo apt-get update && (sudo apt-get install -y valgrind || sleep 5 && sudo apt-get install -y valgrind) && valgrind --version
- run: sudo apt update && (sudo apt install -y valgrind || sleep 5 && sudo apt install -y valgrind) && valgrind --version

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While apt and apt-get do overlap in functionality, apt is not intended for script usage and makes fewer/no interface compatibility guarantees compared to underlying tools like apt-get (https://unix.stackexchange.com/questions/590699/should-i-use-apt-or-apt-get-in-shell-scripting, https://serverfault.com/questions/958003/how-to-disable-warning-apt-does-not-have-a-stable-cli-interface, https://manpages.ubuntu.com/manpages/xenial/man8/apt.8.html).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, thanks for sharing! I definitely did not know this, and I guess have been making the mistake of using apt in scripts for a while now. Oops 😅

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.85%. Comparing base (2209a7c) to head (b8e4ea8).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3953   +/-   ##
=======================================
  Coverage   97.85%   97.85%           
=======================================
  Files        1303     1303           
  Lines       78139    78140    +1     
  Branches     3894     3894           
=======================================
+ Hits        76461    76463    +2     
+ Misses       1678     1677    -1     
Flag Coverage Δ
97.85% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pr-commenter
Copy link

pr-commenter bot commented Sep 25, 2024

Benchmarks

Benchmark execution time: 2024-09-25 17:33:25

Comparing candidate commit b8e4ea8 in PR branch retry-memory-leaks with baseline commit 2209a7c in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 23 metrics, 2 unstable metrics.

@TonyCTHsu TonyCTHsu merged commit 9e1ff92 into master Sep 26, 2024
196 checks passed
@TonyCTHsu TonyCTHsu deleted the retry-memory-leaks branch September 26, 2024 10:31
@TonyCTHsu TonyCTHsu added the dev/tooling Involves tools (e.g. Rubocop, CodeCov) label Sep 26, 2024
@github-actions github-actions bot added this to the 2.4.0 milestone Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev/tooling Involves tools (e.g. Rubocop, CodeCov)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants