Skip to content

Commit 4066e9e

Browse files
committed
fix: multiple bug fixes across cli, api, runner and dockerfile
1 parent df2d0a2 commit 4066e9e

8 files changed

Lines changed: 36 additions & 29 deletions

File tree

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
__pycache__
22
AGENTS.md
3+
TASKS.md

Dockerfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ USER nonroot
4747
WORKDIR /app
4848

4949
# Defaults (overridable at runtime)
50-
ENV PIPELINE_FILE=/app/pipelines/example_pipeline.yaml
50+
ENV PIPELINE_FILE=/app/pipelines/example_pipeline_simple.yaml
5151
ENV DOCKER_BASE_URL=unix:///var/run/docker.sock
5252

5353
# Run the application using uv which uses the uv-managed venv

src/pipeline_scheduler/application/runner.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -105,10 +105,6 @@ def run_single_step(step: StepModel) -> tuple[bool, int | None]:
105105
f"Image {step.image} not present locally (get() failed), attempting pull"
106106
)
107107
pull_image(client, step.image)
108-
logger.info(
109-
f"Image {step.image} not present locally, pulling"
110-
)
111-
pull_image(client, step.image)
112108
elif pull_policy == "never":
113109
logger.debug(
114110
f"pull_policy=never, skipping pull for {step.image}"

src/pipeline_scheduler/application/scheduler.py

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,6 @@ def start_scheduler(config: AppConfig, pipeline: PipelineModel):
3737
scheduler = BlockingScheduler()
3838

3939
def job_func():
40-
# Concurrency guard using shared state
41-
if state.running.get("job"):
42-
logger.warning("Previous job still running, skipping this run")
43-
return
44-
4540
job_id = str(uuid.uuid4())
4641

4742
# Build initial JobModel for scheduler
@@ -56,7 +51,11 @@ def job_func():
5651
steps=steps,
5752
)
5853

54+
# Concurrency guard: check and set atomically under the lock
5955
with state.jobs_lock:
56+
if state.running.get("job"):
57+
logger.warning("Previous job still running, skipping this run")
58+
return
6059
state.jobs[job_id] = job
6160
state.running["job"] = job_id
6261

src/pipeline_scheduler/domain/models.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,10 @@ class AppConfig(BaseModel):
103103
retry_on_fail: int = 0
104104
step_timeout: int = 0
105105
log_level: str = "INFO"
106+
api_enabled: bool = True
106107
api_host: str = "0.0.0.0"
107108
api_port: int = 8080
109+
api_key_header: str = "X-API-Key"
108110

109111

110112
# State models for in-memory job tracking (used by API and scheduler)

src/pipeline_scheduler/infrastructure/templating.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ def render_pipeline(
2929
merged.update(params)
3030

3131
# Always use strict undefined behavior: missing variables raise errors.
32-
env = Environment(undefined=StrictUndefined, autoescape=True)
32+
env = Environment(undefined=StrictUndefined, autoescape=False)
3333
template = env.from_string(content)
3434
rendered = template.render(**merged)
3535
obj = yaml.safe_load(rendered)

src/pipeline_scheduler/interfaces/api.py

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
render_tree_ascii,
2424
)
2525

26-
API_KEY_HEADER_NAME = "X-API-Key"
26+
API_KEY_HEADER_NAME = os.getenv("API_KEY_HEADER", "X-API-Key")
2727
api_key_header = APIKeyHeader(name=API_KEY_HEADER_NAME, auto_error=False)
2828

2929
# Module-level AppConfig instance to be set by the server so API handlers
@@ -74,7 +74,11 @@ async def trigger(payload: Dict[str, Any], api_key: str = Depends(get_api_key)):
7474
"""Trigger the pipeline run..."""
7575

7676
# Merge parameters: start from config.pipeline_params then overlay payload pipeline_params
77-
assert CONFIG is not None, "CONFIG must be set before handling requests"
77+
if CONFIG is None:
78+
raise HTTPException(
79+
status_code=http_status.HTTP_500_INTERNAL_SERVER_ERROR,
80+
detail="Server misconfiguration: CONFIG not initialized",
81+
)
7882
config_params = dict(CONFIG.pipeline_params or {})
7983
config_params.update(payload.get("pipeline_params", {}))
8084

@@ -91,12 +95,6 @@ async def trigger(payload: Dict[str, Any], api_key: str = Depends(get_api_key)):
9195
detail="API trigger disabled for this pipeline",
9296
)
9397

94-
# concurrency guard
95-
if state.running.get("job"):
96-
raise HTTPException(
97-
status_code=http_status.HTTP_409_CONFLICT, detail="Pipeline already running"
98-
)
99-
10098
job_id = str(uuid.uuid4())
10199

102100
# Build initial JobModel with per-step entries
@@ -112,8 +110,12 @@ async def trigger(payload: Dict[str, Any], api_key: str = Depends(get_api_key)):
112110
steps=steps,
113111
)
114112

115-
# store job under lock and mark running guard
113+
# Concurrency guard: check and set atomically under the lock
116114
with state.jobs_lock:
115+
if state.running.get("job"):
116+
raise HTTPException(
117+
status_code=http_status.HTTP_409_CONFLICT, detail="Pipeline already running"
118+
)
117119
state.jobs[job_id] = job
118120
state.running["job"] = job_id
119121

@@ -125,7 +127,6 @@ def _done_cb(f):
125127
ok = f.result()
126128
with state.jobs_lock:
127129
j: JobModel | None = state.jobs.get(job_id)
128-
assert j is not None, "JobModel should exist in state.jobs"
129130
if j:
130131
j.status = "success" if ok else "failed"
131132
if j.ended_at is None:
@@ -134,7 +135,6 @@ def _done_cb(f):
134135
except Exception:
135136
with state.jobs_lock:
136137
j: JobModel | None = state.jobs.get(job_id)
137-
assert j is not None, "JobModel should exist in state.jobs"
138138
if j:
139139
j.status = "error"
140140
if j.ended_at is None:
@@ -187,7 +187,11 @@ async def show(
187187
return {"tree": sp.model_dump_json(), "text": text}
188188

189189
# static view from configured pipeline
190-
assert CONFIG is not None, "CONFIG must be set before handling requests"
190+
if CONFIG is None:
191+
raise HTTPException(
192+
status_code=http_status.HTTP_500_INTERNAL_SERVER_ERROR,
193+
detail="Server misconfiguration: CONFIG not initialized",
194+
)
191195
try:
192196
raw = render_pipeline(
193197
path=CONFIG.pipeline_file, params=CONFIG.pipeline_params or {}

src/pipeline_scheduler/interfaces/cli.py

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,10 @@ def build_config(
2626
retry: Optional[int] = None,
2727
step_timeout: Optional[int] = None,
2828
log_level: Optional[str] = None,
29+
api_enabled: Optional[bool] = None,
2930
api_port: Optional[int] = None,
3031
api_host: Optional[str] = None,
32+
api_key_header: Optional[str] = None,
3133
) -> AppConfig:
3234
"""Construct AppConfig from CLI parameters and environment variables.
3335
@@ -69,11 +71,16 @@ def _int_or(value, fallback: int) -> int:
6971
step_timeout if step_timeout is not None else env.get("STEP_TIMEOUT"), 0
7072
),
7173
log_level=(log_level or env.get("LOG_LEVEL") or "INFO"),
72-
# pipeline params removed from AppConfig; they must be specified in YAML
74+
api_enabled=(
75+
api_enabled
76+
if api_enabled is not None
77+
else env.get("API_ENABLED", "true").lower() not in ("0", "false")
78+
),
7379
api_host=(api_host or env.get("API_HOST") or "0.0.0.0"),
7480
api_port=_int_or(
7581
api_port if api_port is not None else env.get("API_PORT"), 8080
7682
),
83+
api_key_header=(api_key_header or env.get("API_KEY_HEADER") or "X-API-Key"),
7784
)
7885
# log when using bundled example pipeline (no CLI arg and no PIPELINE_FILE env)
7986
if not pipeline_path and not env.get("PIPELINE_FILE"):
@@ -111,6 +118,7 @@ def main(
111118
retry=retry,
112119
step_timeout=step_timeout,
113120
log_level=log_level,
121+
api_enabled=api_enabled,
114122
api_port=api_port,
115123
api_host=api_host,
116124
)
@@ -121,7 +129,7 @@ def main(
121129

122130
# render pipeline (params must be defined within the YAML file itself)
123131
try:
124-
raw = render_pipeline(config.pipeline_file)
132+
raw = render_pipeline(config.pipeline_file, params=config.pipeline_params)
125133
except Exception:
126134
logger.exception("Failed to load/render pipeline file %s", config.pipeline_file)
127135
raise SystemExit(2)
@@ -148,9 +156,6 @@ def main(
148156
print(render_tree_ascii(sp=sp, color=True))
149157
return
150158

151-
# Decide mode based on environment: API vs CLI
152-
api_enabled = os.getenv("API_ENABLED", "true").lower() not in ("0", "false")
153-
154159
# Determine if pipeline has a schedule (new 'schedule')
155160
schedule_expr = getattr(pipeline.metadata, "schedule", None)
156161
# Respect RUN_ONCE env var if set and CLI flag not given
@@ -171,7 +176,7 @@ def main(
171176
logger.exception("Failed to run pipeline once")
172177
raise SystemExit(3)
173178

174-
if api_enabled:
179+
if config.api_enabled:
175180
# start server which will look at pipeline.metadata.schedule to decide scheduling
176181
try:
177182
server.main(

0 commit comments

Comments
 (0)