fix: raise consistent ResourceNotFoundError when project cannot be resolved#510
fix: raise consistent ResourceNotFoundError when project cannot be resolved#510thiagobomfin-galileo wants to merge 1 commit intomainfrom
Conversation
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
tests/future/test_log_stream.py
Outdated
| # When/Then: create() raises ResourceNotFoundError with helpful message | ||
| with pytest.raises(ResourceNotFoundError, match="Project not found"): | ||
| log_stream.create() | ||
|
|
There was a problem hiding this comment.
AGENTS.md requires Given/When/Then in sentence case; can we capitalize # When/Then: create() raises ResourceNotFoundError… to 'Create() raises…'?
Finding type: AI Coding Guidelines | Severity: 🟢 Low
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
In tests/future/test_log_stream.py around lines 170 to 173, the comment for the
When/Then block in the test_create_without_project_info_raises_error method starts with
a lowercase 'create()' which violates the project's Given/When/Then sentence-case
guideline. Edit the comment so it starts with an uppercase first word: change '#
When/Then: create() raises ResourceNotFoundError with helpful message' to '# When/Then:
Create() raises ResourceNotFoundError with helpful message'.
There was a problem hiding this comment.
Commit 061bcaf addressed this comment by updating the When/Then comment to sentence case so it now reads “Create() raises…” as requested.
tests/future/test_log_stream.py
Outdated
| # When/Then: calling get raises ResourceNotFoundError | ||
| with pytest.raises(ResourceNotFoundError, match="Project not found"): | ||
| LogStream.get(name="Test Stream") |
There was a problem hiding this comment.
AGENTS.md requires sentence-case Given/When/Then — should we change # When/Then: calling get raises ResourceNotFoundError to # When/Then: Calling get raises ResourceNotFoundError?
Finding type: AI Coding Guidelines | Severity: 🟢 Low
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
In tests/future/test_log_stream.py around lines 274 to 276, inside the
test_get_raises_error_without_project_info_and_no_env_fallback method, the inline
comment `# When/Then: calling get raises ResourceNotFoundError` uses lowercase 'calling'
and breaks the sentence-case testing guideline. Replace that comment with sentence-case
wording, e.g. `# When/Then: Calling get raises ResourceNotFoundError`, preserving
punctuation and placement.
There was a problem hiding this comment.
Commit 061bcaf addressed this comment by updating the inline When/Then comment to sentence case (# When/Then: Calling get raises ResourceNotFoundError), matching the requested style.
tests/future/test_log_stream.py
Outdated
| # When/Then: calling list raises ResourceNotFoundError | ||
| with pytest.raises(ResourceNotFoundError, match="Project not found"): | ||
| LogStream.list() |
There was a problem hiding this comment.
AGENTS.md requires sentence-case Given/When/Then — should we capitalize # When/Then: calling list raises ResourceNotFoundError to # When/Then: Calling list raises ResourceNotFoundError?
Finding type: AI Coding Guidelines | Severity: 🟢 Low
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
In tests/future/test_log_stream.py around lines 396 to 398, the comment for the
When/Then clause in test_list_raises_error_without_project_info_and_no_env_fallback
begins with a lowercase word: "# When/Then: calling list raises ResourceNotFoundError"
which violates the sentence-case testing style. Change the comment to start with a
capitalized word, e.g. "# When/Then: Calling list raises ResourceNotFoundError". Ensure
punctuation and capitalization follow the project's AGENTS.md testing guidelines.
There was a problem hiding this comment.
Commit 061bcaf addressed this comment by capitalizing the When/Then clause comment so it now reads “Calling list raises ResourceNotFoundError,” matching the sentence-case style.
| # Resolve project using explicit params or env fallbacks (GALILEO_PROJECT_ID, GALILEO_PROJECT) | ||
| project_obj = Projects().get_with_env_fallbacks(id=self.project_id, name=self.project_name) | ||
| if not project_obj: | ||
| raise ValueError("Project not found. Provide project_id, project_name, or set GALILEO_PROJECT env var.") | ||
| raise ResourceNotFoundError( | ||
| "Project not found. Provide project_id, project_name, or set GALILEO_PROJECT env var." | ||
| ) |
There was a problem hiding this comment.
get_with_env_fallbacks raises ValueError instead of returning None, so Experiment.create never reaches its ResourceNotFoundError branch—should we catch ValueError and re-raise as ResourceNotFoundError or return None from the helper?
Finding type: Logical Bugs | Severity: 🔴 High
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
In src/galileo/__future__/experiment.py around lines 380 to 385, the create method calls
Projects().get_with_env_fallbacks(...) but that helper can raise ValueError when both id
and name are None, so the new ResourceNotFoundError branch is never reached. Modify the
create method to wrap the Projects().get_with_env_fallbacks(...) call in a try/except
that catches ValueError and raises ResourceNotFoundError with the existing message
("Project not found. Provide project_id, project_name, or set GALILEO_PROJECT env
var."). This ensures callers receive ResourceNotFoundError in the no-project/no-env
scenario. If you prefer a different approach, instead update
Projects.get_with_env_fallbacks to return None rather than raising when neither id nor
name is supplied and update callers accordingly.
There was a problem hiding this comment.
- Analysis: This is partially correct but overstated. I verified
get_with_env_fallbacksinsrc/galileo/projects.py:158-195. It delegates toProjects.get()which raisesValueError("Exactly one of 'id' or 'name' must be provided.")only when bothidandnameresolve toNone(no params, no env vars). When a user passes a project name/id that simply doesn't exist,get()returnsNoneand theResourceNotFoundErrorbranch IS reached. TheValueErrorpath only triggers in the edge case where the caller provides no project info AND no env vars are set — in that scenario, the user would still get aValueErrorfrom the underlyingget()method rather than the newResourceNotFoundError. This is a real gap but low severity — the error is still informative and the primary use case (user passes a non-existent project name) works correctly. - Recommended action: Consider wrapping the
get_with_env_fallbackscall in atry/except ValueErrorto convert it toResourceNotFoundErrorfor full consistency, but this is a follow-up improvement, not a blocker.
There was a problem hiding this comment.
Thanks for the detailed clarification on the get_with_env_fallbacks behavior and the suggested try/except improvement—this is noted and I’ll save it to memory after the PR is merged.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #510 +/- ##
==========================================
+ Coverage 82.34% 82.36% +0.02%
==========================================
Files 103 103
Lines 9412 9412
==========================================
+ Hits 7750 7752 +2
+ Misses 1662 1660 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
thiagobomfin-galileo
changed the title
| # Resolve project using explicit params or env fallbacks (GALILEO_PROJECT_ID, GALILEO_PROJECT) | ||
| project_obj = Projects().get_with_env_fallbacks(id=self.project_id, name=self.project_name) | ||
| if not project_obj: | ||
| raise ValueError("Project not found. Provide project_id, project_name, or set GALILEO_PROJECT env var.") | ||
| raise ResourceNotFoundError( |
There was a problem hiding this comment.
Projects().get_with_env_fallbacks(...) and its ResourceNotFoundError handling are duplicated in this file, should we extract _resolve_project_or_raise(project_id, project_name) to wrap the call, raise ResourceNotFoundError on None, and return the resolved project?
Finding type: Code Dedup and Conventions | Severity: 🟢 Low
Want Baz to fix this for you? Activate Fixer
There was a problem hiding this comment.
- Analysis: The duplication exists across three files (log_stream, experiment, prompt), not just within one file. Extracting a helper is reasonable but is a refactoring concern outside the scope of this bug fix PR. The current approach is simple and consistent. Per CLAUDE.md: "Avoid over-engineering. Only make changes that are directly requested or clearly necessary."
- Recommended action: No action needed for this PR. Could be a follow-up if desired.
There was a problem hiding this comment.
Understood, I’ll treat this as out-of-scope for the current bug fix and keep the existing approach, and I’ll save this preference about avoiding overengineering and cross-file refactors for future reviews once the PR is merged.
…esolved Replaces inconsistent ValueError raises with the SDK-owned ResourceNotFoundError across LogStream, Experiment, and Prompt domain objects, consistent with the __future__ exception hierarchy. fix(__future__): raise consistent ResourceNotFoundError when project cannot be resolved
thiagobomfin-galileo
force-pushed
the
sc-52684-future__.LogStream.list()
branch
from
061bcaf to
832fabc
Compare
2 participants
User description
Replace inconsistent ValueError with ResourceNotFoundError across all future domain objects (LogStream, Experiment, Prompt) when a project cannot be resolved, so users get a clear, consistent exception
regardless of whether they pass project_name or project_id.
Also clean up stale imports from galileo.future.shared.exceptions to use the canonical galileo.shared.exceptions path after the shared/ folder was moved to root.
Shortcut:
https://app.shortcut.com/galileo/story/52684/future-logstream-list-raises-unclear-typeerror-when-project-does-not-exist
Description:
Tests:
Verify all ResourceNotFoundError assertions pass in test_log_stream.py, test_experiment.py, test_prompt.py
Run LogStream.list(project_name="non-existent") and confirm ResourceNotFoundError is raised with an actionable message
Confirm mypy passes with no issues (Success: no issues found in 89 source files)
Generated description
Below is a concise technical summary of the changes proposed in this PR:
Ensure
LogStream,Experiment, andPromptnow raiseResourceNotFoundErrorwhen project resolution fails so the SDK surfaces a consistent, clear failure message regardless of whether the caller usedproject_idorproject_name. Switch shared exception imports to the canonicalgalileo.shared.exceptionspath so both source and tests stay aligned with the relocated shared package layout.galileo.shared.exceptionsso all modules reference the canonical path after the shared/ folder relocation.Modified files (6)
Latest Contributors(2)
ResourceNotFoundErrorinLogStream,Experiment, andPromptcreate/get/list flows when the project cannot be resolved, update the documentedRaisessections, and extend the future tests to assert the clearer error state (including the new prompt scenario).Modified files (6)
Latest Contributors(2)