-
Notifications
You must be signed in to change notification settings - Fork 120
core:services:helper: Migrate helper to secondary venv and update its dependencies #3729
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
base: master
Are you sure you want to change the base?
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideMigrates the helper service from the primary core workspace into the secondary python-venv2 environment and refreshes its dependency stack (FastAPI, Pydantic, etc.), including lockfiles and startup wiring, with one minor model default tweak for compatibility. Updated class diagram for ServiceInfo model default changeclassDiagram
class ServiceMetadata
class ServiceInfo {
+str name
+List~str~ versions
+int port
+Optional~str~ path
+Optional~ServiceMetadata~ metadata = None
+int __hash__()
}
ServiceInfo --> ServiceMetadata
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
* Update all libraries to latest * Move helper to venv2
* Allow ServiceInfo model to receive optional metadata field to avoid validation error
ee1e1f9 to
2030971
Compare
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.
Hey - I've left some high level feedback:
- With the bump to FastAPI 0.125 and Pydantic 2.12, review helper service code for any Pydantic v1-style APIs (e.g.,
.dict(),.json(),Configclasses,arbitrary_types_allowed) and FastAPI behaviors that changed (e.g., response_model handling,jsonable_encoder) to ensure they’re updated to the v2 patterns. - Now that
metadataonServiceInfois explicitly defaulted toNone, verify any callers that relied on it being required or always present so they don’t break onNoneand adjust type hints or runtime checks accordingly.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- With the bump to FastAPI 0.125 and Pydantic 2.12, review helper service code for any Pydantic v1-style APIs (e.g., `.dict()`, `.json()`, `Config` classes, `arbitrary_types_allowed`) and FastAPI behaviors that changed (e.g., response_model handling, `jsonable_encoder`) to ensure they’re updated to the v2 patterns.
- Now that `metadata` on `ServiceInfo` is explicitly defaulted to `None`, verify any callers that relied on it being required or always present so they don’t break on `None` and adjust type hints or runtime checks accordingly.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
API calls can be validated at
http://blueos.local:81/v1.0/docsSummary by Sourcery
Migrate the helper service into the secondary Python virtual environment and align its configuration and dependencies with the updated workspace layout.
Bug Fixes:
Enhancements: