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

Support DOCKER_REGISTRY_URL env var #8

Closed

Conversation

msabramo
Copy link
Contributor

@msabramo msabramo commented Jul 7, 2016

If the environment variable DOCKER_REGISTRY_URL is set, then the value of it will be used as the default registry URL, instead of "https://index.docker.io".

E.g.:

$ export DOCKER_REGISTRY_URL=https://docker.corp.surveymonkey.com
$ docker-ls repositories
requesting list from https://docker.corp.surveymonkey.com . done
repositories:
- acme/webhook
- admintools/admintools-nodejs-asset-build
- cx
...

If the environment variable `DOCKER_REGISTRY_URL` is set, then the value
of it will be used as the default registry URL, instead of
`"https://index.docker.io"`.
Now that the `DOCKER_REGISTRY_URL` env var can change the registry URL,
it is more useful to see the registry URL.
@zoobab
Copy link

zoobab commented Aug 23, 2016

Any idea when this is gonna merged/refused? This PR is already more then one month old...

@DirtyHairy
Copy link
Member

Hi @msabramo !

Sorry for taking so long to respond to your PR. The code looks mostly fine to me; however, there's some error handling missing in initRegistryUrl. Specifically, you don't test for parse errors during url.Parse, leading to a potentially uninitialized url struct. I would prefer to change DEFAULT_REGISTRY_URL and Config.registryUrl to pointers that are set to nil in this case. This way, the client can check whether it has a valid (non-nil) url at startup and throw an error in the vein of "no valid URL configured" otherwise.

Would you mind making these changes?

Cheers
-Christian

@zoobab
Copy link

zoobab commented Nov 10, 2016

This is the most useful feature for me.

@DirtyHairy
Copy link
Member

DirtyHairy commented Sep 21, 2017

v0.3.0 supports configuring the registry via DOCKER_LS_REGISTRY. Still, thanks for your work @msabramo

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.

None yet

3 participants