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

Fix docker rate limit #184

Merged

Conversation

UnmeshDeodhar
Copy link
Contributor

Changing registry to a fixed version so that we will not hit the docker hub rate limit.

Fixes: #182

Copy link
Member

@fidencio fidencio left a comment

Choose a reason for hiding this comment

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

@UnmeshDeodhar, although the patch is good, the commit message is not.

I'd suggest changing the commit message to something like:

tests: Use a specific version of the registry container

Switch to a fixed version of the registry containers, so a new version is not pulled
in every time a test runs, avoiding then hitting the docker hub rate limit.

@UnmeshDeodhar
Copy link
Contributor Author

/test

1 similar comment
@wainersm
Copy link
Member

/test

@fidencio
Copy link
Member

@UnmeshDeodhar, almost there, but it's still a very long line.

Mind to break it as shown below?

Switch to a fixed version of the registry containers, so a new version
is not pulled in every time a test runs, avoiding then hitting the
docker hub rate limit.

Switch to a fixed version of the registry containers, so a new version
is not pulled in every time a test runs, avoiding then hitting the
docker hub rate limit.

Fixes: confidential-containers#182

Signed-off-by: Unmesh Deodhar <udeodhar@amd.com>
@UnmeshDeodhar
Copy link
Contributor Author

/test

Copy link
Member

@wainersm wainersm left a comment

Choose a reason for hiding this comment

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

Thanks @UnmeshDeodhar !

@wainersm
Copy link
Member

/test

Copy link
Member

@fidencio fidencio left a comment

Choose a reason for hiding this comment

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

lgtm, thanks @UnmeshDeodhar!

@wainersm wainersm merged commit f5dc74c into confidential-containers:main Mar 27, 2023
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.

tests/e2e: hitting docker.io/library/registry pull rate limit
3 participants