Gracefully handle missing driver client imports #142
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughA StubDriverClient fallback is added for unavailable drivers; import failures now surface as MissingDriverError during class import and are optionally suppressed via an env var. Client initialization falls back to the stub and invoking stub methods raises a standardized ImportError with installation/version guidance. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@python/packages/jumpstarter/jumpstarter/client/base.py`:
- Around line 118-126: Change _get_missing_class_path to defensively access the
"jumpstarter.dev/client" label (e.g., use
self.labels.get("jumpstarter.dev/client")) and return a sensible fallback or
None; then update _raise_missing_error to handle a missing class_path by
constructing and raising the ImportError with a clear message (via
_format_missing_driver_message or a new message) instead of allowing a KeyError.
Ensure references are in _get_missing_class_path and _raise_missing_error so
malformed reports produce the intended ImportError with installation
instructions.
In `@python/packages/jumpstarter/jumpstarter/common/importlib.py`:
- Around line 66-76: The current ModuleNotFoundError handler masks transitive
dependency failures; change it to only convert to MissingDriverError when the
missing name matches the target module (or a submodule). Extract the module
portion from class_path (e.g., module_name = class_path.rsplit('.', 1)[0]) and
in the except ModuleNotFoundError as e block, if e.name == module_name or
e.name.startswith(module_name + ".") then raise
MissingDriverError(message=_format_missing_driver_message(class_path),
class_path=class_path) from e; otherwise re-raise the original
ModuleNotFoundError to preserve transitive dependency errors; keep the existing
AttributeError -> MissingDriverError behavior intact.
🧹 Nitpick comments (2)
python/packages/jumpstarter/jumpstarter/client/base.py (1)
15-15: Consider making the helper function public or moving it to a shared location.
_format_missing_driver_messageis a private function (underscore prefix) being imported from another module. While this works, it couplesbase.pyto the internal implementation ofimportlib.py. Consider either removing the underscore to make it a public API or moving it to a shared utilities module.python/packages/jumpstarter/jumpstarter/config/exporter.py (1)
47-52: Consider preserving the exception chain for debugging.Using
from Nonesuppresses the original exception chain, which can make debugging harder when diagnosing why a driver import failed. Consider preserving the cause.Suggested change
except MissingDriverError: raise ConfigurationError( f"Driver '{self.root.type}' is not installed. Please check exporter configuration." - ) from None + )Or if you want a cleaner traceback while still showing the root cause:
except MissingDriverError as e: raise ConfigurationError( f"Driver '{self.root.type}' is not installed. Please check exporter configuration." ) from e
evakhoni
force-pushed
the
import/python-pr-805
branch
2 times, most recently
from
0e3663b to
95faca4
Compare
|
I have tried with a local jumpstarter install with not SNMP driver: I suspect we may want to just grab any call on the client for catching other non call/stream methods clients may have. But I think that is better, I would merge it now, and iterate on that a continuation. |
evakhoni
force-pushed
the
import/python-pr-805
branch
from
95faca4 to
e68ae6a
Compare
mangelajo
left a comment
mangelajo
left a comment
There was a problem hiding this comment.
See my comment on the main conversation, that will need iteration, let's wait for the python tests to pass before trying to merge.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@python/packages/jumpstarter/jumpstarter/client/client.py`:
- Around line 59-66: The StubDriverClient should surface a helpful ImportError
whenever driver attributes or methods are accessed; modify StubDriverClient in
jumpstarter/client/base.py to accept and store the missing driver identifier
(e.g., class_path or driver name) when constructed, and implement __getattr__
(and if needed __getitem__) so any unknown attribute access immediately raises
ImportError with a clear message pointing to the missing driver identifier and
guidance, or alternatively return a proxy callable that raises that ImportError
when invoked; ensure the constructor signature matches how StubDriverClient is
instantiated from client.py so the missing driver info is available for the
error message.
@mangelajo |
2 participants
Imported from jumpstarter-dev/jumpstarter-python#805
closes: #79
this PR fixes an issue where a client that is unable to load the full set of drivers exported (for example with exporter version newer than the client) is prevented form connecting to said exporter at all. the issue is remedied with the following changes:
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.