stabilize docker compose orchestration and networking logic#545
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the CLI build wrapper to generate more usable Docker Compose output for mkconcore-generated Docker workflows, aiming to make multi-container networking and startup behavior work out-of-the-box.
Changes:
- Generate a
docker-compose.ymlthat defines and attaches all services to a sharedconcore-netbridge network. - Add
restart: on-failure, a sequentialdepends_onchain, and emit a top-levelvolumes:block for detected named volumes. - Copy a
requirements.txtintoout/src/and patch generated build scripts to include it in each Docker build context.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -104,10 +107,30 @@ def _write_docker_compose(output_path): | |||
| compose_lines.append( | |||
| f" container_name: {_yaml_quote(service['container_name'])}" | |||
| ) | |||
| compose_lines.append(" restart: on-failure") | |||
| compose_lines.append(" networks:") | |||
| compose_lines.append(" - concore-net") | |||
|
|
|||
| # Chain services sequentially to prevent ZMQ race conditions | |||
| if prev_service_name: | |||
| compose_lines.append(" depends_on:") | |||
| compose_lines.append(f" - {prev_service_name}") | |||
There was a problem hiding this comment.
service_name is emitted as an unquoted YAML key and referenced unquoted in depends_on. If a container name starts with a digit (or looks like a number, e.g. "1", "1.0"), YAML parsers can treat it as a numeric scalar, and Docker Compose unmarshalling expects string service keys. Consider quoting the service keys (and the depends_on entries) or forcing a non-numeric prefix when the first char is not a letter to ensure Compose always reads these as strings.
There was a problem hiding this comment.
Good catch! I've updated the logic to use .isalpha() instead of .isalnum() to strictly enforce safe string prefixes, and applied _yaml_quote directly to the top-level service definitions to prevent any numeric parser crashes.
Note: I actually ended up removing the depends_on chaining entirely. Since we are using restart: on-failure, Docker inherently handles the ZMQ readiness loop, meaning the linear depends_on overhead was just needlessly serializing our architecture's concurrent startup times!
|
@pradeeban this should be ready for your review, if you have any suggestions please do let me know |
|
@pradeeban, also just a question, do we show any interest for concore implementation in scilab? |
|
We support both MATLAB and Octave. So why not? But I also think these extra implementations are going to be just research curiosity rather than a real need. So perhaps not. |
|
for the past week I was completing an assignment related to converting the 'control systems' octave functions to scilab functions for a fellowship (Academic), that just got me interested to ask! |
|
@pradeeban Also it would be cool if we can explore any possibilities of integrating machine learning with concore systems after the complete GSoC phase :) , I assume that would increase the practical applications even more. |
|
@GREENRAT-K405 Actually, there were several machine learning projects that were part of the CONTROL-CORE project. If you go over @mayureshkothare's Google Scholar profile, you will come across some. |
Thank you, @pradeeban! I’ll take a look at @mayureshkothare’s Google Scholar profile. |
I have fixed the primary networking disconnect preventing docker-compose generated workflows from functioning out of the box.
While fixing this network isolation, I have also fixed three secondary problems (startup race conditions, missing volume declarations, and missing Python contexts) entirely from within the new CLI wrapper (concore_cli/commands/build.py).
I have avoided modifying mkconcore.py for this one!
Before:
After: