Closed andreycizov closed 5 years ago
I like this very much, awesome work ! 😃
I have a few suggestions :
labelPrefix should be fr.dldl.sphinx-server
instead of com.dldl.sphinx-server
as dldl.fr
is registered and owned by @quentinus95 , and dldl.com
is not.
I think it would be better to add logic to the containers port numbers, ( for example with something like find-port ) as right now it just starts adding up from 54334
and doesn't check if the port is already used, which could cause unfriendly errors.
Finally, I agree that we should get @Coac 's input on a container destruction/cleanup strategy. Maybe atom provides hooks for the plugin when it closes ?
So now we use find-free-port
to find a free port, then put it into the container label. It allows for a better interaction between separate windows of Atom: e.g. if you've got the same folder opened in two windows at the same time, they'll reuse the same docker container in order to build it.
@andreycizov , thanks for all this work 🙂 !
@Coac , @quentinus95 , do you have access to a windows machine to test this ?
@Coac , I suggest that we create a new issue and PR for the container cleanup strategy, and merge this first, if you are okay with that.
There's one thing to discuss: how do we manage the destruction of the created containers?
I am leaving this up to the maintainer, but currently all containers created are assigned the same label, so that it would be easy to remove them via command line.