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

Use public.ecr.aws images #617

Closed
wants to merge 1 commit into from

Conversation

AndreKurait
Copy link
Member

Description

Currently we are getting throttling from docker and ecr within github actions. To resolve this, we should

TODO: before this change is merged, we should include adding credentials for authenticated ecr pulling to extend our limits.

  • Category: Bug Fix
  • Why these changes are required? Fix throttling for github actions
  • What is the old behavior before changes and new behavior after changes? Some test runs failed and need to be retried.

Issues Resolved

Is this a backport? If so, please add backport PR # and/or commits #

Testing

Tested locally

Check List

  • New functionality includes testing
    • [x ] All tests pass, including unit test, integration test and doctest
  • [x ] New functionality has been documented
  • [x ] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Andre Kurait <[email protected]>
Copy link

codecov bot commented Apr 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.96%. Comparing base (0c6d19d) to head (6e6f0d9).
Report is 3 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #617      +/-   ##
============================================
+ Coverage     75.93%   75.96%   +0.03%     
  Complexity     1496     1496              
============================================
  Files           165      165              
  Lines          6362     6362              
  Branches        573      573              
============================================
+ Hits           4831     4833       +2     
+ Misses         1150     1147       -3     
- Partials        381      382       +1     
Flag Coverage Δ
unittests 75.96% <ø> (+0.03%) ⬆️

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.

@peternied
Copy link
Member

@AndreKurait Is this still needed? Seems like the recent change [1] by @mikaylathompson will addresses the same root problem

@sumobrian
Copy link
Collaborator

Closing. We can reopen if needed.

@sumobrian sumobrian closed this Jul 1, 2024
@gregschohn
Copy link
Collaborator

If the anonymous pull rates are lower for ECR than w/ docker, this solves the problem for us, but not any other users that are trying to build these images in a continuous environment. I'd love to be able to keep using the same identifiers for the images that we want (like grafana) and no be stuck with baking the image repo that works best for any given user into the code. If there was a mirror that we could use instead, that might be the simplest solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants