[Qrytics]

Slot 428 — PR body for nextstrain/cli

Bug

nextstrain.cli.util.split_image_name crashes with ValueError: too many values to unpack on Docker image names whose registry includes a port
(e.g. localhost:5000/nextstrain/base:build-...), because it splits on the
first colon while a valid image reference can contain two. Private
registries with ports (localhost:5000/..., my-registry:5000/...) are
common in local development, so any --docker runtime path using such an
image fails with a traceback.

Repro (before fix):

>>> from nextstrain.cli.util import split_image_name
>>> split_image_name("localhost:5000/nextstrain/base:build-1")
ValueError: too many values to unpack (expected 2)

Fix

nextstrain/cli/util.py (split_image_name): split on the last colon,
and only treat it as a tag separator when it comes after the last slash
(Docker semantics: the repository part may itself contain a colon for a
registry port). localhost:5000/nextstrain/base (no tag) now correctly
resolves to ('localhost:5000/nextstrain/base', 'latest') instead of
crashing, and all previously working inputs behave exactly as before.

Test

Extended the function's doctests (the repo runs --doctest-modules over
nextstrain/) with the registry-port cases. Verified the new cases crash on
the old code (ValueError) and pytest --doctest-modules nextstrain/cli/util.py → 4 passed after the fix.

[victorlin]

Thanks for reporting. I have noticed the bug in certain situations such as nextstrain shell --image localhost:5000/nextstrain/base:latest. A workaround is to drop :latest, but that's tag-specific and actually goes the wrong path in split_image_name() with repository=localhost and tag=5000/nextstrain/base. It doesn't end up mattering much because the function is only called while checking whether the image supports /nextstrain/env.d. The new behavior is an improvement.