Show an error on 'run' when there are legacy one-off containers #1643
Conversation
aanand
force-pushed
the
warn-about-legacy-one-off-containers
branch
3 times, most recently
from
269743d
to
02e228e
Compare
| else: | ||
| legacy_containers.append(container) | ||
|
|
||
| return (legacy_containers, legacy_one_off_containers) |
There was a problem hiding this comment.
I think this implementation could be a little simpler. Instead of always returning both, it could take a one_off=False kwarg (like a bunch of functions do currently).
If it's True return only the one_off containers, of it's False only the non-one_off containers.
That removes the need for the tuple return in check_name, validate_name, and this function.
I removes the unused return value from migrate_project_to_labels, and the logic in check_for_legacy_containers will be about the same.
There was a problem hiding this comment.
I guess the disadvantage there is the extra query for containers, which is relatively slow, but we're already adding one slow query to docker-compose run either way
There was a problem hiding this comment.
Yeah - that was why I did it this way, but it does add a fair bit of complexity. As you say, I don't think it's worth the performance savings in the end. Will redo.
aanand
force-pushed
the
warn-about-legacy-one-off-containers
branch
from
02e228e
to
6b8c5a3
Compare
Also warn the user about the one-off containers in the standard error message about legacy containers. Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>
aanand
force-pushed
the
warn-about-legacy-one-off-containers
branch
from
6b8c5a3
to
e98caf5
Compare
|
OK, I've made the code changes you suggested, and also changed the behaviour of |
|
LGTM |
…ners Show an error on 'run' when there are legacy one-off containers
…containers Show an error on 'run' when there are legacy one-off containers (cherry picked from commit 81707ef) Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>
Also warn the user about the one-off containers in the standard error message about legacy containers.
Closes #1609.