Fix regression in up
#1858
Fix regression in up
#1858
Conversation
When running `docker-compose up A B`, logs are shown for those services *and all their dependencies*, which is not what the user expects. This regressed in bd554a6. Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>
| if service in services: | ||
| containers_to_return += containers | ||
|
|
||
| return containers_to_return |
There was a problem hiding this comment.
I'm not sure about this. I think project.up() should still return all the containers it created.
I would think the filtering should happen in the cli.main.up() (or it can call a function to do the filtering) since that's the layer that deals with user expectations.
I think it's conceivable that something else might want to know all the containers created by up (I say this because I've actually written code that has that expectation).
There was a problem hiding this comment.
Fair enough - only problem with doing it in cli.main.up is that it can't be tested.
There was a problem hiding this comment.
Ya, that's true. At some point I think it might make sense to split up some of the larger cli functions into some smaller functions (to make it easier to test), but that doesn't need to happen now.
In this case I think everything that happens after project.up() could be in a separate function:
if detached:
return
print_logs_for_up(containers, service_names, monochrome, timeout)
|
Closed by #1931 |
3 participants
When running
docker-compose up A B, logs are shown for those services and all their dependencies, which is not what the user expects. This regressed in #1645.Not wild about the variable names here - would appreciate a second pair of eyes.
Closes #1856.