Added control of docker networks of containers#244
Added control of docker networks of containers#244Sharpz7 wants to merge 4 commits intoFNNDSC:masterfrom
Conversation
|
Tested with 2 networks |
|
I haven't ran the tests locally - I imagine they will need some fixes. |
jennydaman
left a comment
There was a problem hiding this comment.
Typically I would say it is courtesy to run the tests before opening a PR, but in this case I can sympathize with how pman's codebase is somewhat finicky and doing so might be inconvenient.
- It seems like supporting multiple Docker networks will be a non-trivial change to the code. Maybe we should support only a single Docker network instead. At startup, if
len(DOCKER_NETWORKS) > 1, raise a configuration error. - Assuming you accept my proposal to limit our scope to supporting only 0–1 Docker networks, I would say just delete the whole
test_dockermgr.pyfile.
|
Appreciate the suggestions @jennydaman I hope you can appreciate that without a I will work on your suggestions in time. Happy to go with the single network route for now and remove tests altogether. |
|
@Sharpz7 100% agree. (If i had the authority to enforce standards and |
|
I apologize for my editor automatically applying formatting to the file - let me know if you would like that undone. |
No description provided.