Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Read container-name from lsp-docker.yml both for image and container … #58

Closed
wants to merge 1 commit into from

Conversation

korenhe
Copy link

@korenhe korenhe commented Aug 31, 2022

…type

For image type, the container-name is nil and makes the name of repeated
creation of container conflicts with each other.

@factyy
Copy link
Collaborator

factyy commented Nov 23, 2022

@korenhe , sorry for the late reply! What about not reading names for images (containers created from them are meant to be only temporary, this is the whole point of using this server type) but randomizing them? As far as I remember there was a function used for adding random suffixes to container names. Maybe integrate it somehow? As for the container names we can just use random strings (as I wrote above the containers are temporary and are deleted when the server stops).

For image type, the container-name is nil and makes the name of repeated
creation of container conflicts with each other. This patch provides a default
name "lsp-container" with an increamental suffix to let multiple containers run
at the same time, the 'docker ps' output is like this:

CONTAINER ID   IMAGE              ...    NAMES
33501e5e3305   testimage:latest   ...    lsp-container-2
e9808f58a29f   testimage:latest   ...    lsp-container-1
@factyy
Copy link
Collaborator

factyy commented May 8, 2023

@korenhe , please take a look at this PR: #70 . It basically enables generation of container names (unique, using a global suffix) under the hood thus avoiding container name clashes.

I will probably close this particular PR in the near future. Thanks for your input!

@factyy
Copy link
Collaborator

factyy commented May 10, 2023

Closing this PR as the underlying issue is now fixed by another PR

@factyy factyy closed this May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants