-
Notifications
You must be signed in to change notification settings - Fork 11
refactor: remove jobs-table dependencies from Python SDK #625
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| from unittest.mock import MagicMock, patch | ||
| from uuid import uuid4 | ||
|
|
||
| import pytest | ||
|
|
||
| from galileo.experiment import Experiment | ||
| from galileo.shared.base import SyncState | ||
| from galileo.shared.experiment_result import ExperimentStatusInfo | ||
|
|
||
| FIXED_PROJECT_ID = str(uuid4()) | ||
| FIXED_EXPERIMENT_ID = str(uuid4()) | ||
|
|
||
|
|
||
| def _make_status(progress_percent: float) -> ExperimentStatusInfo: | ||
| """Build an ExperimentStatusInfo with a given log_generation progress (0-100).""" | ||
| phase = MagicMock() | ||
| phase.progress_percent = progress_percent / 100.0 # API uses 0.0-1.0 | ||
| response = MagicMock() | ||
| response.status.log_generation = phase | ||
| return ExperimentStatusInfo(response) | ||
|
|
||
|
|
||
| def _make_experiment() -> Experiment: | ||
| exp = Experiment._create_empty() | ||
| exp.id = FIXED_EXPERIMENT_ID | ||
| exp.project_id = FIXED_PROJECT_ID | ||
| exp.name = "test-experiment" | ||
| exp._set_state(SyncState.SYNCED) | ||
| return exp | ||
|
|
||
|
|
||
| class TestMonitorProgress: | ||
| @patch("galileo.experiment.Experiment.get_status") | ||
| @patch("galileo.experiment.sleep", return_value=None) | ||
| def test_completes_when_status_reaches_100(self, mock_sleep, mock_get_status): | ||
| # Given: an experiment that progresses through 0%, 50%, then 100% | ||
| mock_get_status.side_effect = [_make_status(0.0), _make_status(50.0), _make_status(100.0)] | ||
| exp = _make_experiment() | ||
|
|
||
| # When: monitoring progress until completion | ||
| exp.monitor_progress(poll_interval_seconds=0.0) | ||
|
|
||
| # Then: get_status is polled until 100% is reached | ||
| assert mock_get_status.call_count == 3 | ||
|
|
||
| @patch("galileo.experiment.Experiment.get_status") | ||
| @patch("galileo.experiment.sleep", return_value=None) | ||
| def test_already_complete_on_first_poll(self, mock_sleep, mock_get_status): | ||
| # Given: an experiment that is already at 100% on the first poll | ||
| mock_get_status.return_value = _make_status(100.0) | ||
| exp = _make_experiment() | ||
|
|
||
| # When: monitoring progress | ||
| exp.monitor_progress(poll_interval_seconds=0.0) | ||
|
|
||
| # Then: get_status is called once and sleep is never called | ||
| assert mock_get_status.call_count == 1 | ||
| mock_sleep.assert_not_called() | ||
|
|
||
| @patch("galileo.experiment.Experiment.get_status") | ||
| @patch("galileo.experiment.sleep", return_value=None) | ||
| def test_uses_poll_interval_seconds(self, mock_sleep, mock_get_status): | ||
| # Given: an experiment that completes on the second poll | ||
| mock_get_status.side_effect = [_make_status(0.0), _make_status(100.0)] | ||
| exp = _make_experiment() | ||
|
|
||
| # When: monitoring with a custom poll interval | ||
| exp.monitor_progress(poll_interval_seconds=5.0) | ||
|
|
||
| # Then: sleep is called once with the specified interval | ||
| mock_sleep.assert_called_once_with(5.0) | ||
|
|
||
| def test_raises_without_experiment_id(self): | ||
| # Given: an experiment without an id | ||
| exp = Experiment._create_empty() | ||
| exp.id = None | ||
| exp.project_id = FIXED_PROJECT_ID | ||
|
|
||
| # When/Then: monitoring raises ValueError about the missing experiment id | ||
| with pytest.raises(ValueError, match="Experiment ID is not set"): | ||
| exp.monitor_progress() | ||
|
|
||
| def test_raises_without_project_id(self): | ||
| # Given: an experiment without a project_id | ||
| exp = Experiment._create_empty() | ||
| exp.id = FIXED_EXPERIMENT_ID | ||
| exp.project_id = None | ||
|
|
||
| # When/Then: monitoring raises ValueError about the missing project id | ||
| with pytest.raises(ValueError, match="Project ID is not set"): | ||
| exp.monitor_progress() | ||
|
|
||
| @patch("galileo.experiment.Experiment.get_status") | ||
| @patch("galileo.experiment.sleep", return_value=None) | ||
| def test_deprecated_job_id_warns(self, mock_sleep, mock_get_status): | ||
| # Given: an experiment that is already complete, and a caller passing the deprecated job_id | ||
| mock_get_status.return_value = _make_status(100.0) | ||
| exp = _make_experiment() | ||
|
|
||
| # When/Then: monitor_progress emits a DeprecationWarning when job_id is supplied | ||
| with pytest.warns(DeprecationWarning, match="job_id"): | ||
| exp.monitor_progress(job_id="some-old-job-id") |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
monitor_progress()polls forever in two cases: whenstatus.log_generationis absent (defaultingoverall_progressto 0) and when a run stalls below 100% (sinceis_failedalways returnsFalse) — should we fail fast on missinglog_generationand break on explicit failure/cancelled states, plus add atimeout/max_pollsguard?Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents