refactor: remove jobs-table dependencies from Python SDK - #625
Conversation
Delete job_progress.py and jobs.py — both relied on the Postgres jobs table, which is being retired per the jobs-table-decomposition RFC. Rewrite Experiment.monitor_progress() to poll get_status() instead, which reads ExperimentResponse.status.log_generation from the experiment endpoint (no jobs table dependency). The job_id parameter is removed; progress is tracked as a 0-100 tqdm bar over the log_generation phase. get_job(), job_progress(), get_run_scorer_jobs(), and scorer_jobs_status() were all internal-only (not exported from __init__.py) so there is no public API break. Rename test_job_progress.py -> test_experiment_progress.py and rewrite tests to cover the new polling logic. Delete test_jobs.py. Co-Authored-By: Claude <noreply@anthropic.com>
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
| def monitor_progress(self, poll_interval: float = 2.0) -> None: | ||
| """ | ||
| Monitor the progress of the experiment job with a progress bar. | ||
| Monitor the progress of the experiment with a progress bar. | ||
|
|
||
| Args: | ||
| job_id: Optional job ID to monitor. If not provided, will attempt to find | ||
| the primary job for this experiment. | ||
| Polls the experiment status via the API until the experiment completes, | ||
| displaying a tqdm progress bar reflecting `log_generation` progress. | ||
|
|
||
| Returns | ||
| ------- | ||
| str: The unique identifier of the completed job. | ||
| Args: | ||
| poll_interval: Seconds to wait between status polls. Defaults to 2.0. |
There was a problem hiding this comment.
Experiment.monitor_progress drops job_id and its str return in favor of poll_interval: float, so external callers (re-exported via galileo.__init__) that pass job_id= hit a signature mismatch and code expecting a completed job id back now gets None — should we keep a backward-compatible alias or version this break explicitly, and also rename poll_interval to poll_interval_seconds as AGENTS.md requires?
Want Baz to fix this for you? Activate Fixer You can also update your AI coding guidelines based on this comment by apply pr to [branch name]
Other fix methods
Prompt for AI Agents
In src/galileo/experiment.py around lines 1127-1135 in `Experiment.monitor_progress`,
address two issues together: 1. **Breaking signature change**: Revert/bridge the removal
of `job_id` and the `str` return type. Make `monitor_progress` backward-compatible by
accepting `job_id` as an optional keyword (preserving positional support if feasible).
If `job_id` is provided, use the existing `job_progress`/`get_run_scorer_jobs` logic and
return the completed `job_id` string; otherwise use the polling/progress-bar flow and
return `None`. Add a deprecation warning for `job_id` usage so external callers know to
migrate, and ensure the `galileo.__init__` re-export doesn't expose an incompatible
contract without documentation. 2. **Units naming convention**: Rename the
`poll_interval: float = 2.0` parameter to `poll_interval_seconds`, update the docstring
and all internal references, and update any call sites/tests using
`monitor_progress(poll_interval=...)`. If renaming would break a published API, add a
backward-compatible keyword alias and deprecate `poll_interval`, ensuring the primary
documented parameter name includes `_seconds`.
There was a problem hiding this comment.
@john-weiler can you look at this ... the contract looks changed for moniter
There was a problem hiding this comment.
Commit a97c7ea addressed this comment by renaming poll_interval to poll_interval_seconds and updating the docstring/call flow to use the new unit-suffixed name. It also added a deprecated job_id keyword with a warning, though it no longer preserves the old return value behavior.
| while not status.is_complete: | ||
| new_progress = status.overall_progress | ||
| progress_bar.update(new_progress - progress_bar.n) | ||
| sleep(poll_interval) | ||
| status = self.get_status() |
There was a problem hiding this comment.
monitor_progress() polls forever in two cases: when status.log_generation is absent (defaulting overall_progress to 0) and when a run stalls below 100% (since is_failed always returns False) — should we fail fast on missing log_generation and break on explicit failure/cancelled states, plus add a timeout/max_polls guard?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
In `src/galileo/experiment.py` around lines 1089-1168, fix `monitor_progress` to exit
deterministically: 1. In `get_status` (or `ExperimentStatusInfo` construction), detect
when `ExperimentResponse.status` or `status.log_generation` is absent/Unset and raise a
clear `ValueError` (e.g., "Experiment status.log_generation missing; cannot monitor
progress reliably") instead of defaulting to a synthetic 0% phase. 2. In the polling
loop, break on additional terminal signals from the status payload
(failed/cancelled/stopped) and raise a clear exception when detected; update
`ExperimentStatusInfo.is_failed` so completion/failure is derived from explicit backend
run states rather than only `log_generation.progress_percent >= 100`. 3. Add a `timeout`
or `max_polls` guard so the loop exits deterministically even if the backend never
reaches 100%.
| @patch("time.sleep", return_value=None) | ||
| def test_completes_when_status_reaches_100(self, mock_sleep, mock_get_status): | ||
| mock_get_status.side_effect = [ | ||
| _make_status(0.0), | ||
| _make_status(50.0), | ||
| _make_status(100.0), |
There was a problem hiding this comment.
@patch("time.sleep") leaves the module-local sleep in galileo.experiment unmocked, so test_uses_poll_interval still waits 5 real seconds — should we patch galileo.experiment.sleep instead across all three tests? Also, the new test blocks skip the # Given/# When/# Then comments that AGENTS.md requires — should we add those behavioral comments to each test?
Want Baz to fix this for you? Activate Fixer You can also update your AI coding guidelines based on this comment by apply pr to [branch name]
Other fix methods
Prompt for AI Agents
In `tests/test_experiment_progress.py` around lines 33-74, within `TestMonitorProgress`
(tests: `test_completes_when_status_reaches_100`, `test_already_complete_on_first_poll`,
`test_uses_poll_interval`, `test_raises_without_experiment_id`,
`test_raises_without_project_id`): 1. Replace `@patch("time.sleep", return_value=None)`
with `@patch("galileo.experiment.sleep", return_value=None)` in all three sleep-patching
tests, so the module-local `sleep` imported in `src/galileo/experiment.py` is correctly
intercepted. Ensure `mock_sleep.assert_not_called()` /
`assert_called_once_with(poll_interval)` still match the updated mock target. 2.
Refactor each test to include human-readable `# Given: ...`, `# When: ...`, and `# Then:
...` comments (or a combined `# When/Then: ...` for exception-assertion tests)
describing the setup, the call to `exp.monitor_progress`, and the assertions/raised
errors. Keep all existing mocking, side_effect, and assertion logic unchanged — only
add the required comment blocks to match the AGENTS.md repository style.
There was a problem hiding this comment.
Commit a97c7ea addressed this comment by switching the sleep patch target to galileo.experiment.sleep in the three polling tests, so the module-local import is mocked correctly. It also added the requested # Given / # When / # Then style comments, including # When/Then for the exception tests, to match the repository test style.
BipinShetty
left a comment
BipinShetty
left a comment
There was a problem hiding this comment.
@john-weiler please check the baz comments... we can drop the apis not required but supported apis contract should be backward compatible ...
| def monitor_progress(self, poll_interval: float = 2.0) -> None: | ||
| """ | ||
| Monitor the progress of the experiment job with a progress bar. | ||
| Monitor the progress of the experiment with a progress bar. | ||
|
|
||
| Args: | ||
| job_id: Optional job ID to monitor. If not provided, will attempt to find | ||
| the primary job for this experiment. | ||
| Polls the experiment status via the API until the experiment completes, | ||
| displaying a tqdm progress bar reflecting `log_generation` progress. | ||
|
|
||
| Returns | ||
| ------- | ||
| str: The unique identifier of the completed job. | ||
| Args: | ||
| poll_interval: Seconds to wait between status polls. Defaults to 2.0. |
There was a problem hiding this comment.
@john-weiler can you look at this ... the contract looks changed for moniter
- Remove dead `import galileo.jobs` and 8 `@patch.object(galileo.jobs.Jobs, "create")` decorators from test_experiments.py — Jobs.create is no longer called by run_experiment - Fix @patch target in test_experiment_progress.py: use `galileo.experiment.sleep` (the module-local binding) instead of `time.sleep` so sleep is actually mocked - Rename `poll_interval` → `poll_interval_seconds` per AGENTS.md duration naming convention - Add deprecated `job_id` kwarg to monitor_progress() with DeprecationWarning for backward compatibility (old signature was `monitor_progress(job_id=None) -> str`) - Add # Given/When/Then behavioral comments to all tests per AGENTS.md style - Add test_deprecated_job_id_warns to cover new deprecation path Co-Authored-By: Claude <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #625 +/- ##
==========================================
+ Coverage 83.37% 83.53% +0.16%
==========================================
Files 126 124 -2
Lines 11113 11029 -84
==========================================
- Hits 9265 9213 -52
+ Misses 1848 1816 -32 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| Args: | ||
| poll_interval_seconds: Seconds to wait between status polls. Defaults to 2.0. | ||
| job_id: Deprecated. This parameter is ignored; it existed in a prior version | ||
| that polled the jobs table, which has been retired. |
There was a problem hiding this comment.
Public docstring violates numpy policy
monitor_progress()'s docstring still uses Args: and describes a job-id return even though the method now returns None, so it no longer matches AGENTS.md's numpy convention or the actual behavior — should we rewrite it in numpy format and add a Returns section for None?
Want Baz to fix this for you? Activate Fixer You can also update your AI coding guidelines based on this comment by apply pr to [branch name]
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
src/galileo/experiment.py around lines 1128–1166, rewrite the public
`Experiment.monitor_progress()` docstring to comply with the numpy docstring convention.
Replace the current `Args:` block with a `Parameters` section (including
`poll_interval_seconds` and deprecated `job_id`), and add a matching `Returns` section
documenting the new behavior for `-> None` (e.g., explicitly state it returns nothing /
no value). Ensure the docstring no longer implies the old job-id return contract and
that the description of `job_id` being ignored aligns with the deprecation warning and
the signature.
There was a problem hiding this comment.
Commit aab5ce9 addressed this comment by converting monitor_progress()'s docstring from Args: to a NumPy-style Parameters section and adding a Returns section that दस्तcribes None. It also keeps the deprecated job_id note aligned with the current ignored-parameter behavior.
Rewrite the docstring with Parameters/Returns/Raises sections to match AGENTS.md's numpy docstring policy. The previous version used Google-style Args: and omitted a Returns section despite the -> None signature. Co-Authored-By: Claude <noreply@anthropic.com>
2 participants
User description
Summary
job_progress.pyandjobs.py— both relied on the Postgres jobs table, which is being retired per the jobs-table-decomposition RFCExperiment.monitor_progress()to pollget_status()(readsExperimentResponse.status.log_generationfromGET /experiments/{id}) — no jobs-table dependencyjob_idparameter is removed; progress is a 0-100 tqdm bar driven bylog_generation.progress_percentget_job(),job_progress(),get_run_scorer_jobs(),scorer_jobs_status()were all internal-only (not exported from__init__.py) — no public API breaktest_job_progress.py→test_experiment_progress.pywith rewritten tests; deletetest_jobs.pyTest plan
tests/test_experiment_progress.pycovers the newmonitor_progresspolling logicjob_progressorjobsmodulesExperiment.monitor_progress()works end-to-end against a real experiment🤖 Generated with Claude Code
Generated description
Below is a concise technical summary of the changes proposed in this PR:
Refactor
Experimentto monitor progress directly from experiment status, usingget_status()andlog_generation.progress_percentto drive atqdmbar without any jobs-table dependency. Remove the retiredjob_progressandJobshelpers and update the experiment and progress tests to validate the new polling flow end to end.Experiment.monitor_progress()to poll experiment status directly, warn on deprecatedjob_id, and render completion with atqdmprogress bar fromlog_generationprogress.Modified files (2)
Latest Contributors(0)
Modified files (2)
Latest Contributors(0)
Modified files (1)
Latest Contributors(0)
Experimentpreviously depended on.Modified files (2)
Latest Contributors(0)