connecting new ai-lab container to existing docker-db network #92
Conversation
|
We also need to cover the case when a new AI-Lab container sees running Docker-DB. The UI suggests to do nothing if the user wants to use old data. And that's not good. |
| @@ -61,7 +61,7 @@ def bring_itde_up(conf: Secrets) -> None: | |||
| db_info = env_info.database_info | |||
| container_info = db_info.container_info | |||
|
|
|||
| _add_current_container_to_db_network(container_info) | |||
| _add_current_container_to_db_network(container_info.network_info.network_name) | |||
There was a problem hiding this comment.
I like narrow interfaces: 👍
| network.connect(container.id) | ||
|
|
||
|
|
||
| def _is_current_container_not_in_db_network(network_name: str) -> bool: |
There was a problem hiding this comment.
How does this function relate to self._add_current_container_to_db_network()?
I have the feeling that part of self._add_current_container_to_db_network() can be replaced by using this function?
There was a problem hiding this comment.
One is checking if the AI-Lab container needs to be connected to the Docker-DB network. The other one connects the container to the network if the container exists and is not connected to this network yet.
| @@ -127,47 +146,60 @@ def _get_ipv4_ddresses(): | |||
|
|
|||
| def is_itde_running(conf: Secrets) -> Tuple[bool, bool]: | |||
| """ | |||
| Checks if the ITDE container exists and if it is running. Returns the two boolean | |||
| flags - (exists, running). | |||
| Checks if the ITDE container exists and ready to be used. In the Docker Edition that | |||
There was a problem hiding this comment.
Would it be possible to replace the tuple by an enum?
class ItdeContainerStatus(Enum):
EXISTS = 0
RUNNING = 1This would also enable to handle the case not container_name or not network_name more explicitly by returning None or having an explicit enum value UNKNOWN or INVALID.
There was a problem hiding this comment.
I agree, but this would also require changes in the upstream code of the the AI-Lab.
There was a problem hiding this comment.
I moved the out-of-scope-findings to #93
Co-authored-by: Christoph Kuhnke <[email protected]>
Co-authored-by: Nicola Coretti <[email protected]>
| raise RuntimeError('Unable to find the name of the Docker container.') | ||
| network_name = conf.get(AILabConfig.itde_network) | ||
| if not container_name or not network_name: | ||
| raise RuntimeError('Unable to find the name of the Docker container or its network.') |
There was a problem hiding this comment.
| raise RuntimeError('Unable to find the name of the Docker container or its network.') | |
| raise RuntimeError('Unable to find the name of the Docker DB container or its network.') |
closes #89