Skip to content

Commit d625c43

Browse files
authored
Merge pull request #9 from miciav/codex/review-codebase-and-identify-issues
Document open issues from repository review
2 parents e746b33 + f437e10 commit d625c43

1 file changed

Lines changed: 18 additions & 0 deletions

File tree

docs/open_issues.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# Open Issues Identified During Codebase Review
2+
3+
## 1. Scheduler setup is incomplete in `Orchestrator`
4+
* `Orchestrator.__init__` falls back to calling `StatusManager()` without arguments, but the manager requires a database path. Constructing an orchestrator without a `status_manager` or `db_path` will raise a `TypeError` before the service can start. 【F:src/maestro/server/internals/orchestrator.py†L56-L72】【F:src/maestro/server/internals/status_manager.py†L53-L69】
5+
* Methods such as `schedule_dag` expect a `self.scheduler` attribute, yet the orchestrator never instantiates a `BackgroundScheduler`. Any attempt to schedule a DAG will crash with an `AttributeError`. 【F:src/maestro/server/internals/orchestrator.py†L112-L120】
6+
7+
## 2. Logging hooks target the wrong logger names
8+
`Orchestrator.run_dag` installs database log handlers on logger names under the `maestro.core.executors` namespace, but the executors live under `maestro.server.internals.executors`. As a result, task logs never reach the database-backed handler. 【F:src/maestro/server/internals/orchestrator.py†L255-L293】【F:src/maestro/server/internals/executors/factory.py†L1-L23】
9+
10+
## 3. Missing DAG file path accessors in `StatusManager`
11+
* The API and orchestrator rely on `StatusManager.get_dag_filepath`, but the method is not implemented anywhere in the class, leading to runtime errors when starting or scheduling stored DAGs. 【F:src/maestro/server/app.py†L209-L219】【F:src/maestro/server/internals/orchestrator.py†L134-L149】【F:src/maestro/server/internals/status_manager.py†L358-L387】
12+
* Storing the DAG definition through `save_dag_definition` does not persist the originating file path, so even if the missing accessor were added there would be no value to return.
13+
14+
## 4. Incorrect signature usage for `save_dag_definition`
15+
`create_dag_definition` passes the DAG file path into `StatusManager.save_dag_definition`, but the method only accepts the DAG instance. This raises a `TypeError` for every call to the `/dags` endpoint. The implementation likely needs to accept and persist the path for later retrieval. 【F:src/maestro/server/app.py†L179-L198】【F:src/maestro/server/internals/status_manager.py†L358-L365】
16+
17+
## 5. Test suite cannot run with the documented defaults
18+
Running `pytest` fails before collection because required third-party dependencies (`requests`, `responses`, `typer`, `fastapi`, etc.) are missing and the `maestro` package itself is not importable until it is installed. Documented setup instructions should ensure the extras in `pyproject.toml` are installed (for example `pip install -e .[test]`) before invoking the tests. 【ace130†L1-L73】【F:pyproject.toml†L26-L62】

0 commit comments

Comments
 (0)