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

Correctly extract image name when using custom hub. #824

Merged
merged 1 commit into from
Jan 18, 2016

Conversation

tomwilkie
Copy link
Contributor

Fixes #820

@paulbellamy
Copy link
Contributor

I'd like to see a few tests for ones which are already without a version.

@paulbellamy
Copy link
Contributor

also, that regex is awful.

@tomwilkie
Copy link
Contributor Author

I'd like to see a few tests for ones which are already without a version.

AFAICT the docker API will never give you images without versions. This regex expects a version.

also, that regex is awful.

Awfully good? :-) But seriously, whats awful about it? I might remove the names matches, but otherwise it seems pretty minimal and robust.

@paulbellamy
Copy link
Contributor

Also, images don't have to have a slash in them, do they?

@2opremio
Copy link
Contributor

Also, images don't have to have a slash in them, do they?

Very true, ubuntu, busybox etc ...

@paulbellamy
Copy link
Contributor

This regex expects a version.

One of the assumptions I had when writing #752, was that this function is idempotent. It shouldn't break anything, but would be nice to maintain that if possible.

@tomwilkie
Copy link
Contributor Author

Very true, ubuntu, busybox etc ...

Actually those ones do (they come from library/busybox)

But images you make locally won't, so that does need fixing. Unfortunately I think that makes parsing non-deterministic (how to differentiate from hostname/foo vs org/foo). I guess something like "first split on slash, if 2 -> first is org, if 3 -> first is repo" might work?

One of the assumptions I had when writing #752, was that this function is idempotent. It shouldn't break anything, but would be nice to maintain that if possible.

I see - so you want versions to be optional. Thats totally possible.

@tomwilkie tomwilkie self-assigned this Jan 15, 2016
@tomwilkie tomwilkie force-pushed the 820-custom-image-port branch from 3488087 to 984fd52 Compare January 15, 2016 20:41
@tomwilkie
Copy link
Contributor Author

@paulbellamy PTAL

@tomwilkie tomwilkie removed their assignment Jan 15, 2016
@paulbellamy
Copy link
Contributor

LGTM

tomwilkie added a commit that referenced this pull request Jan 18, 2016
Correctly extract image name when using custom hub.
@tomwilkie tomwilkie merged commit 23fd123 into master Jan 18, 2016
@tomwilkie tomwilkie deleted the 820-custom-image-port branch January 18, 2016 13:39
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.

3 participants