-
Notifications
You must be signed in to change notification settings - Fork 584
feat(ContainerClient): add proxy utility #533
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
Conversation
|
@jglogan Could you please take a look? |
|
@mayooot - thanks for the PR. From reading the original issue discussion, I think the direction suggested was to add the utility in github.com/apple/containerization since we'll also want to address this there, and then use that utility in container? |
Ok, thanks for your reply. Do you have any suggestions on where to place this utility? |
@mayooot The issue in the containerization repo is apple/containerization#255. Having it there could allow us to do the right thing for image downloads as well as kernel. I'd start by having a look at the issue (see the comment at apple/containerization#255 (comment)) and then adapting what's in your PR to that. IMO ContainerizationExtras is the best place to put it the proxy utility. Then this PR would just contain your kernel download fix. I'll create another issue for the image pull/push fix in container so that we can keep this PR small and quick to merge. |
|
Added #578 for image push/pull. |
Thanks a lot, I'll check it out ASAP. |
|
For this PR, this will probably need to be fixed: container/.github/workflows/common.yml Line 70 in cb2f424
Might even be good to add a little fuzzing in the git workflow so that it randomly chooses HTTPS_PROXY or https_proxy. |
|
@mayooot Have a go at an updated version once we merge #606. You should be able to bring in your proxy utility then. Did you want to have a crack at apple/containerization#255 and then the container updates to use the affected image store APIs? |
- Makes available the proxy utility from containerization#288. ## Type of Change - [ ] Bug fix - [x] New feature - [ ] Breaking change - [ ] Documentation update ## Motivation and Context The proxy utility allows forward progress on #533. ## Testing - [x] Tested locally - [ ] Added/updated tests - [ ] Added/updated docs
Sure, I can take a look. |
fix: #466