BrainLesion / BrainLesion/BraTS

Decouple Docker/Singularity backend infrastructure

Open
#162 0 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
Python
Stars
85
Forks
19
Avg merge
13h 22m
Merged PRs (30d)
5

Description

### Summary

`docker.py` and `singularity.py` are tightly coupled, have latent bugs, and duplicate logic. This issue covers extracting shared code, fixing import-time side effects, and adding type safety.

### Steps

**1. Extract shared helpers into `brats/core/_backend_shared.py`**

`singularity.py:15-26` imports 8 private (underscore-prefixed) functions from `docker.py`. These are shared concerns (volume mappings, device handling, sanity checks, algorithm info logging) that both backends need. Move them to a shared module. Both backends import from `_backend_shared.py` instead of from each other.

**2. Lazy-init Docker clients (fixes latent `NameError`)**

`docker.py:27-33` and `singularity.py:29-37` initialize Docker clients at module scope:

```python
try:
client = docker.from_env()
except DockerException as e:
logger.error(...)
```

`docker.py` has a bug: when `docker.from_env()` fails, `client` is never assigned — not even to `None`. Any subsequent call referencing `client` raises `NameError`. `singularity.py` handles this correctly by setting `docker_client = None` in the except block. Move initialization into a lazy function or `cached_property` in both modules.

**3. Move year-based branching to data model**

Both `docker.py:452` and `singularity.py:197` duplicate:

```python
if algorithm.meta.year <= 2024:
volume_mappings = _get_volume_mappings_mlcube(...)
else:
volume_mappings = _get_volume_mappings_docker_only(...)
```

These branches select between the MLCube volume layout and the native `/input`/`/output` layout. Add a property to `RunArgs` (e.g. `use_mlcube_layout: bool`) that encodes this. Both backends read the property instead of branching on year.

**4. Add a `BackendRunner` Protocol**

`brats_algorithm.py:169` uses `Callable[..., object]` for the backend dispatch dict — no type safety. Define a `typing.Protocol` specifying the `run_container` signature. This would also expose that `singularity.run_container` has an extra `overlay_size` parameter not present in `docker.run_container`.

Contributor guide

Open the contributing guide

Research direction

Start by reading brats/docker.py, brats/singularity.py, and brats_algorithm.py, focusing on the referenced helper imports, client initialization, year-based volume selection, and backend dispatch. Done means shared helpers live in brats/core/_backend_shared.py, clients initialize lazily, RunArgs exposes the layout choice, and BackendRunner provides a typed dispatch contract for both backends.

Written by the indexing model from the issue text.

Assessment

Tech stack
docker, python
Domain
backend, devops
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.