Skip to content

Commit 0aa594b

Browse files
committed
apply review suggestions for compose stability and test coverage
1 parent f96a1b7 commit 0aa594b

2 files changed

Lines changed: 47 additions & 19 deletions

File tree

concore_cli/commands/build.py

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -97,18 +97,17 @@ def _write_docker_compose(output_path):
9797
"services:",
9898
]
9999

100-
prev_service_name = None
101100
named_volumes = set()
102101
for index, service in enumerate(services, start=1):
103102
service_name = re.sub(r"[^A-Za-z0-9_.-]", "-", service["container_name"]).strip(
104103
"-."
105104
)
106105
if not service_name:
107106
service_name = f"service-{index}"
108-
elif not service_name[0].isalnum():
107+
elif not service_name[0].isalpha():
109108
service_name = f"service-{service_name}"
110109

111-
compose_lines.append(f" {service_name}:")
110+
compose_lines.append(f" {_yaml_quote(service_name)}:")
112111
compose_lines.append(f" image: {_yaml_quote(service['image'])}")
113112
compose_lines.append(
114113
f" container_name: {_yaml_quote(service['container_name'])}"
@@ -117,11 +116,6 @@ def _write_docker_compose(output_path):
117116
compose_lines.append(" networks:")
118117
compose_lines.append(" - concore-net")
119118

120-
# Chain services sequentially to prevent ZMQ race conditions
121-
if prev_service_name:
122-
compose_lines.append(" depends_on:")
123-
compose_lines.append(f" - {prev_service_name}")
124-
125119
if service["volumes"]:
126120
compose_lines.append(" volumes:")
127121
for volume_spec in service["volumes"]:
@@ -130,7 +124,6 @@ def _write_docker_compose(output_path):
130124
if re.match(r"^[a-zA-Z0-9_-]+$", part1):
131125
named_volumes.add(part1)
132126

133-
prev_service_name = service_name
134127

135128
if named_volumes:
136129
compose_lines.append("")
@@ -219,22 +212,24 @@ def build_workflow(
219212
elif (output_path / "src").exists():
220213
req_dest.touch()
221214

222-
# Append requirement copying to generated scripts
215+
# Append requirement copying to generated scripts robustly
223216
for s_name in ["build", "build.bat"]:
224217
s_path = output_path / s_name
225218
if s_path.exists():
226219
content = s_path.read_text(encoding="utf-8")
220+
lines = content.splitlines()
227221
if s_name == "build":
228-
content = content.replace(
229-
"docker build",
230-
"cp ../src/requirements.txt .\ndocker build",
231-
)
222+
insert_line = "cp ../src/requirements.txt ."
232223
else:
233-
content = content.replace(
234-
"docker build",
235-
"copy ..\\src\\requirements.txt .\n docker build",
236-
)
237-
s_path.write_text(content, encoding="utf-8")
224+
insert_line = "copy ..\\src\\requirements.txt ."
225+
226+
new_lines = []
227+
for line in lines:
228+
if " build" in line and "-t " in line:
229+
new_lines.append(insert_line)
230+
new_lines.append(line)
231+
232+
s_path.write_text("\n".join(new_lines) + "\n", encoding="utf-8")
238233

239234
if result.stdout:
240235
console.print(result.stdout)

tests/test_cli.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,10 @@ def test_build_command_docker_compose_single_node(self):
383383
self.assertIn("services:", compose_content)
384384
self.assertIn("container_name: 'N1'", compose_content)
385385
self.assertIn("image: 'docker-script'", compose_content)
386+
self.assertIn("networks:", compose_content)
387+
self.assertIn("concore-net:", compose_content)
388+
self.assertIn("- concore-net", compose_content)
389+
self.assertIn("restart: on-failure", compose_content)
386390

387391
metadata = json.loads(Path("out/STUDY.json").read_text())
388392
self.assertIn("docker-compose.yml", metadata["checksums"])
@@ -431,6 +435,35 @@ def test_build_command_docker_compose_multi_node(self):
431435
self.assertIn("container_name: 'C'", compose_content)
432436
self.assertIn("image: 'docker-common'", compose_content)
433437

438+
def test_build_command_docker_requirements_injection(self):
439+
with self.runner.isolated_filesystem(temp_dir=self.temp_dir):
440+
result = self.runner.invoke(cli, ["init", "test-project"])
441+
self.assertEqual(result.exit_code, 0)
442+
443+
Path("requirements.txt").write_text("pandas==1.0.0")
444+
445+
result = self.runner.invoke(
446+
cli,
447+
[
448+
"build",
449+
"test-project/workflow.graphml",
450+
"--source",
451+
"test-project/src",
452+
"--output",
453+
"out",
454+
"--type",
455+
"docker",
456+
],
457+
)
458+
self.assertEqual(result.exit_code, 0)
459+
460+
req_path = Path("out/src/requirements.txt")
461+
self.assertTrue(req_path.exists())
462+
self.assertEqual(req_path.read_text(), "pandas==1.0.0")
463+
464+
build_script = Path("out/build").read_text()
465+
self.assertIn("cp ../src/requirements.txt .", build_script)
466+
434467
def test_build_command_shared_source_specialization_merges_edge_params(self):
435468
with self.runner.isolated_filesystem(temp_dir=self.temp_dir):
436469
Path("src").mkdir()

0 commit comments

Comments
 (0)