remove trailing slash from registries #32
Conversation
|
Can one of the admins verify this patch?
|
|
Ping @baude. Haven't touched python for a while 😃 |
registries/registries.py
Outdated
| f.write(data) | ||
|
|
||
|
|
||
| def trim_suffix(s, suffix): |
There was a problem hiding this comment.
Just use rstrip.
python
Python 2.7.14 (default, Feb 27 2018, 20:43:24)
[GCC 7.3.1 20180130 (Red Hat 7.3.1-2)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> s="This is a test///"
>>> s.rstrip("/")
'This is a test'
There was a problem hiding this comment.
rstrip would remove any number of trailing slashes, but we remove at most one (see https://github.com/vrothberg/image/blob/1c2d9a32e63ab66cb512803622a9e0c414e89381/pkg/sysregistries/system_registries_test.go#L42). Should we adjust that in containers/image as well?
There was a problem hiding this comment.
I see no reason why you would ever have a url of a registry that ends with a slash, so removing only one versus all seems like the wrong solution.
There was a problem hiding this comment.
I updated this PR to use str.rstrip(). Shall I update containers/image to strip all as well?
There was a problem hiding this comment.
|
LGTM |
|
@baude PTAL |
|
could you add a test or two ? |
This is a follow-up change for containers/image#428. It is a common pitfall to add a trailing slash when specifying a registry (e.g., "docker.io/library/" version "docker.io/library"). Remove trailing slashes to normalize the representation of registries. To facilitate testing, don't overwrite the BINARY and MIGRATOR variable when being set in test.bats. Signed-off-by: Valentin Rothberg <vrothberg@suse.com>
|
@baude I just extended the bats tests. I did not manage to get the tests running with the Makefile, so I made the BINARY and MIGRATOR variables configurable for the bats test. Tests are passing on my machine. If I find some time, I can look into enabling Travis. |
|
ping @baude |
|
LGTM! |
|
📌 Commit 2e9862b has been approved by |
|
☀️ Test successful - status-papr |
4 participants
This is a follow-up change for containers/image#428.
It is a common pitfall to add a trailing slash when specifying a
registry (e.g., "docker.io/library/" version "docker.io/library").
Remove trailing slash to normalize the representation of registries.
Signed-off-by: Valentin Rothberg vrothberg@suse.com