Skip to content

Commit b40a138

Browse files
authored
Fix streaming docker logs (#3)
* streaming docker logs * enable pushing docker images from PRs * removing build ghcr * allowing only one cbio import at a time * added more formatting & checks to ci * added pyrefly typehecks
1 parent a7da1fe commit b40a138

22 files changed

Lines changed: 211 additions & 195 deletions

.github/workflows/ci.yml

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,32 +2,47 @@ name: CI
22

33
on:
44
push:
5-
branches: ["main"]
5+
branches: ["**"]
66
pull_request:
77
branches: ["**"]
88

99
jobs:
1010
lint:
1111
runs-on: ubuntu-latest
12+
1213
container:
1314
image: ghcr.io/astral-sh/uv:python3.12-bookworm-slim
15+
1416
steps:
1517
- uses: actions/checkout@v6
18+
1619
- name: Install dependencies
1720
run: uv sync --all-extras
21+
1822
- name: Lint
1923
run: uv run ruff check
2024

25+
- name: Format check
26+
run: uv run ruff format --check
27+
28+
- name: Type check
29+
run: uv run pyrefly check
30+
2131
test:
2232
runs-on: ubuntu-latest
33+
2334
container:
2435
image: ghcr.io/astral-sh/uv:python3.12-bookworm-slim
36+
2537
steps:
2638
- uses: actions/checkout@v6
39+
2740
- name: Install dependencies
2841
run: uv sync --all-extras
42+
2943
- name: Test
3044
run: uv run pytest -v --cov=app --cov-report=xml --cov-report=term-missing --cov-fail-under=95
45+
3146
- name: Upload coverage report
3247
uses: actions/upload-artifact@v4
3348
with:
@@ -37,21 +52,36 @@ jobs:
3752
docker:
3853
runs-on: ubuntu-latest
3954
needs: [lint, test]
55+
4056
permissions:
4157
contents: read
4258
packages: write
59+
4360
steps:
4461
- uses: actions/checkout@v6
62+
4563
- name: Log in to GHCR
4664
uses: docker/login-action@v3
4765
with:
4866
registry: ghcr.io
4967
username: ${{ github.repository_owner }}
5068
password: ${{ secrets.GITHUB_TOKEN }}
69+
70+
- name: Docker meta
71+
id: meta
72+
uses: docker/metadata-action@v5
73+
with:
74+
images: ghcr.io/${{ github.repository }}
75+
tags: |
76+
type=ref,event=branch
77+
type=ref,event=pr
78+
type=sha
79+
type=raw,value=latest,enable=${{ github.ref == 'refs/heads/main' }}
80+
5181
- name: Build and push
5282
uses: docker/build-push-action@v6
5383
with:
5484
context: .
5585
file: docker/Dockerfile
56-
push: ${{ github.ref == 'refs/heads/main' }}
57-
tags: ghcr.io/${{ github.repository }}:latest,ghcr.io/${{ github.repository }}:${{ github.sha }}
86+
push: true
87+
tags: ${{ steps.meta.outputs.tags }}

Makefile

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@ format:
1212
.PHONY: check
1313
check:
1414
uv run ruff check .
15+
uv run ruff format --check .
16+
17+
.PHONY: type
18+
type:
19+
uv run pyright
1520

1621
.PHONY: fix
1722
fix:
@@ -41,10 +46,6 @@ worker:
4146
docker:
4247
bash docker/build.sh
4348

44-
.PHONY: docker-ghcr
45-
docker-ghcr:
46-
bash docker/build-ghcr.sh
47-
4849
.PHONY: db-migrate
4950
db-migrate:
5051
uv run alembic upgrade head

app/__init__.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
# cbio-ingest FastAPI application
2-
from dotenv import load_dotenv
32
from importlib.metadata import PackageNotFoundError
43
from importlib.metadata import metadata as _metadata
54

5+
from dotenv import load_dotenv
6+
67
load_dotenv()
78

89
try:

app/fs.py

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,9 @@ def get_fs_service_studies(
6666
session: Session = Depends(get_session),
6767
) -> FileSystemService:
6868
"""Dependency to get filesystem service instance."""
69-
return FileSystemService(
70-
session=session, base_path=os.getenv("STUDY_DIR", "/app/study")
71-
)
69+
return FileSystemService(session=session, base_path=os.getenv("STUDY_DIR", "/app/study"))
7270

7371

7472
def get_fs_service_panels(session: Session = Depends(get_session)) -> FileSystemService:
7573
"""Dependency to get filesystem service instance."""
76-
return FileSystemService(
77-
session=session, base_path=os.getenv("PANEL_DIR", "/app/panel")
78-
)
74+
return FileSystemService(session=session, base_path=os.getenv("PANEL_DIR", "/app/panel"))

app/routers/panels.py

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
from app.auth import verify_token
88
from app.db import get_session
99
from app.fs import FileSystemService, get_fs_service_panels
10-
from app.models import IngestQuery, Panel, Status
10+
from app.models import IngestQuery, Panel, Status, Study
1111
from app.scheduler import queue
1212
from app.tasks import ingest_panel
1313

@@ -22,7 +22,11 @@ async def list_panels(
2222
fs: FileSystemService = Depends(get_fs_service_panels),
2323
token=Depends(verify_token),
2424
) -> list[Panel]:
25-
"""List all ingested panels. Pass `?available` to list panels on disk instead. Pass `?all` to merge both."""
25+
"""List all ingested panels.
26+
27+
Pass `?available` to list panels on disk instead.
28+
Pass `?all` to merge both.
29+
"""
2630
if available is not None:
2731
return fs.list_panels()
2832
if all is not None:
@@ -33,7 +37,11 @@ async def list_panels(
3337
return list(session.exec(select(Panel)).all())
3438

3539

36-
@router.post("/", status_code=201, responses={400: {"description": "Bad Request"}})
40+
@router.post(
41+
"/",
42+
status_code=201,
43+
responses={400: {"description": "Bad Request"}, 409: {"description": "Conflict"}},
44+
)
3745
async def create_panel(
3846
data: IngestQuery,
3947
keep_logs: bool = Query(default=False),
@@ -65,6 +73,12 @@ async def create_panel(
6573
session.commit()
6674
session.refresh(panel)
6775

76+
if (
77+
session.exec(select(Study).where(Study.status == Status.IN_PROGRESS)).first()
78+
or session.exec(select(Panel).where(Panel.status == Status.IN_PROGRESS)).first()
79+
):
80+
raise HTTPException(status_code=409, detail="Another ingestion is already in progress")
81+
6882
job_timeout = int(os.getenv("JOB_TIMEOUT", "3600"))
6983
job = queue.enqueue(ingest_panel, panel.id, job_timeout=job_timeout)
7084

app/routers/studies.py

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
import os
22

3-
from sqlalchemy.orm import attributes
43
from fastapi import APIRouter, Depends, HTTPException, Query
4+
from sqlalchemy.orm import attributes
55
from sqlmodel import Session, select
66

77
from app.auth import verify_token
88
from app.db import get_session
99
from app.fs import FileSystemService, get_fs_service_studies
10-
from app.models import IngestQuery, Status, Study
10+
from app.models import IngestQuery, Panel, Status, Study
1111
from app.scheduler import queue
1212
from app.tasks import ingest_study
1313

@@ -22,7 +22,11 @@ async def list_studies(
2222
fs: FileSystemService = Depends(get_fs_service_studies),
2323
token=Depends(verify_token),
2424
) -> list[Study]:
25-
"""List all ingested studies. Pass `?available` to list studies on disk instead. Pass `?all` to merge both."""
25+
"""List all ingested studies.
26+
27+
Pass `?available` to list studies on disk instead.
28+
Pass `?all` to merge both.
29+
"""
2630
if available is not None:
2731
return fs.list_studies()
2832
if all is not None:
@@ -33,7 +37,11 @@ async def list_studies(
3337
return list(session.exec(select(Study)).all())
3438

3539

36-
@router.post("/", status_code=201, responses={400: {"description": "Bad Request"}})
40+
@router.post(
41+
"/",
42+
status_code=201,
43+
responses={400: {"description": "Bad Request"}, 409: {"description": "Conflict"}},
44+
)
3745
async def create_study(
3846
data: IngestQuery,
3947
keep_logs: bool = Query(default=False),
@@ -65,6 +73,12 @@ async def create_study(
6573
session.commit()
6674
session.refresh(study)
6775

76+
if (
77+
session.exec(select(Study).where(Study.status == Status.IN_PROGRESS)).first()
78+
or session.exec(select(Panel).where(Panel.status == Status.IN_PROGRESS)).first()
79+
):
80+
raise HTTPException(status_code=409, detail="Another ingestion is already in progress")
81+
6882
job_timeout = int(os.getenv("JOB_TIMEOUT", "3600"))
6983
job = queue.enqueue(ingest_study, study.id, job_timeout=job_timeout)
7084

app/scheduler.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,4 @@
77
redis_port = int(os.getenv("REDIS_PORT", "6379"))
88
redis_password = os.getenv("REDIS_PASSWORD") or None
99

10-
queue = Queue(
11-
connection=Redis(host=redis_host, port=redis_port, password=redis_password)
12-
)
10+
queue = Queue(connection=Redis(host=redis_host, port=redis_port, password=redis_password))

app/tasks.py

Lines changed: 37 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import codecs
12
import os
23
from datetime import UTC, datetime
34

@@ -48,42 +49,59 @@ def _run_ingest(
4849
add_log(entity, LogLevel.INFO, "worker", f"Running command: {' '.join(cmd)}")
4950
print(f"Starting ingestion with command: {' '.join(cmd)} ...")
5051

51-
exec_result = container.exec_run(
52-
cmd=cmd,
53-
workdir=workdir,
52+
entity.command = " ".join(cmd)
53+
session.add(entity)
54+
session.commit()
55+
session.refresh(entity)
56+
57+
exec_id = client.api.exec_create(
58+
container.id,
59+
cmd,
5460
stdout=True,
5561
stderr=True,
56-
stream=False,
62+
workdir=workdir,
63+
environment={"PYTHONUNBUFFERED": "1"},
5764
)
65+
stream = client.api.exec_start(exec_id["Id"], stream=True)
66+
decoder = codecs.getincrementaldecoder("utf-8")()
67+
buffer = ""
68+
69+
for chunk in stream:
70+
text = decoder.decode(chunk)
71+
buffer += text
72+
while "\n" in buffer:
73+
line, buffer = buffer.split("\n", 1)
74+
line = line.rstrip("\r")
75+
76+
if not line.strip():
77+
continue
78+
79+
add_log(entity, LogLevel.INFO, "docker", line)
80+
session.add(entity)
81+
session.commit()
82+
83+
session.add(entity)
84+
session.commit()
85+
session.refresh(entity)
86+
87+
inspect = client.api.exec_inspect(exec_id["Id"])
88+
exit_code = inspect["ExitCode"]
5889

5990
container.reload()
60-
entity.command = " ".join(cmd)
6191
entity.cbioportal_version = (
6292
getattr(container, "attrs", {}).get("Config", {}).get("Image", "unknown")
6393
)
6494
session.add(entity)
6595
session.commit()
6696
session.refresh(entity)
6797

68-
logs = exec_result.output.decode("utf-8").split("\n")
69-
exit_code = exec_result.exit_code
70-
7198
print(f"Finished ingestion with exit code {exit_code} (0 = success)")
7299

73-
log_level = LogLevel.INFO if exit_code == 0 else LogLevel.ERROR
74-
75-
for log in logs:
76-
stripped_log = log.strip()
77-
if stripped_log:
78-
add_log(entity, log_level, "docker", stripped_log)
79-
80100
if exit_code == 0:
81101
print("Restarting container to apply changes ...")
82102
container.restart()
83103
print("Finished restarting container")
84-
add_log(
85-
entity, LogLevel.INFO, "worker", "Container restarted to apply changes."
86-
)
104+
add_log(entity, LogLevel.INFO, "worker", "Container restarted to apply changes.")
87105
mark_completed(entity, session)
88106
entity.date_ingested = datetime.now(UTC)
89107
session.add(entity)
@@ -117,7 +135,7 @@ def ingest_study(study_id: int) -> None:
117135
cmd = [
118136
"metaImport.py",
119137
"-u",
120-
os.getenv("CBIOPORTAL_URL", "http://cbioportal:8080"),
138+
"http://cbioportal:8080",
121139
"-s",
122140
f"/study/{validated_name}",
123141
"-o",

app/tests/test_auth.py

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,13 @@ class TestVerifyToken:
1212

1313
def test_valid_token(self, mock_token: str):
1414
"""Test that valid token is accepted."""
15-
credentials = HTTPAuthorizationCredentials(
16-
scheme="Bearer", credentials=mock_token
17-
)
15+
credentials = HTTPAuthorizationCredentials(scheme="Bearer", credentials=mock_token)
1816
result = verify_token(credentials)
1917
assert result == mock_token
2018

2119
def test_invalid_token(self, mock_token: str):
2220
"""Test that invalid token raises HTTPException."""
23-
credentials = HTTPAuthorizationCredentials(
24-
scheme="Bearer", credentials="wrong-token"
25-
)
21+
credentials = HTTPAuthorizationCredentials(scheme="Bearer", credentials="wrong-token")
2622
with pytest.raises(HTTPException) as exc_info:
2723
verify_token(credentials)
2824

0 commit comments

Comments
 (0)