chore(py): Simplify Dev UI integration and remove unused abstractions#4812
chore(py): Simplify Dev UI integration and remove unused abstractions#4812huangjeff5 wants to merge 11 commits intomainfrom
Conversation
Summary of ChangesHello @huangjeff5, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly simplifies the integration of the Genkit Dev UI, particularly for web applications, by automating the reflection server's startup process. It removes previous complexities and boilerplate code, making the developer experience smoother and the internal codebase more maintainable. The changes streamline how Genkit instances manage their lifecycle and interact with different application types, focusing on a more intuitive 'set it and forget it' approach for development environments. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request significantly simplifies the integration of the Genkit Dev UI by automating the startup of the reflection server in a background daemon thread when running in development mode (GENKIT_ENV=dev). This removes the need for framework-specific boilerplate like genkit_lifespan for FastAPI. The cleanup of unused internal abstractions (web/manager) and dead types improves maintainability. My feedback focuses on improving the efficiency of the health check loop and ensuring consistent binding behavior in the port utility.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request is a great simplification of the Dev UI integration. Moving the reflection server to a background daemon thread with its own event loop is a clean solution that removes a lot of complexity and boilerplate for developers using frameworks like FastAPI. The removal of the web/manager abstraction and related code is a significant improvement for maintainability. The new tests for the background server behavior are thorough and cover the important aspects of the new design, including thread safety.
I have one major piece of feedback regarding a potential race condition in how free ports are selected for the reflection server. Other than that, the changes look solid.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request significantly simplifies the Dev UI integration by refactoring how the reflection server is started. The new approach of using a background daemon thread with its own event loop is a great improvement, as it resolves event loop conflicts with web frameworks like FastAPI and simplifies the user code by removing the need for run_main or framework-specific lifespan hooks in many cases. The removal of the complex web/manager abstraction and the simplification of run_main make the core library cleaner and easier to understand. The introduction of the _loop_local_client helper is a clever solution to ensure that SDK clients are created and used on the correct event loop, and its application across the anthropic, google-genai, and compat-oai plugins is consistent and well-executed. The new tests for the background server, including concurrency tests, are thorough and provide good coverage for the new functionality.
One major aspect of this PR is the removal of a large number of plugins (checks, cloudflare-workers-ai, cohere, deepseek, evaluators, huggingface, mcp). This was not mentioned in the pull request description. While this is a significant change, I've focused my review on the modified code. For future large-scale changes like this, it would be beneficial to document them in the PR description to provide full context.
Overall, the code changes are of high quality and represent a substantial improvement to the framework's developer experience and internal architecture. I have not found any issues of medium or higher severity.
Problem
Previously, run_main() was running reflection server and running your code, both as sync tasks on the same event loop. Web frameworks typically own the event loop and provide their own hook. This means we (as genkit team) have to provide a custom implementation of that hook to support the framework. With FastAPI, we needed to provide a
genkit_lifespan()hook. Genkit users also have to add that hook boilerplate into their code.The reflection server now starts automatically in a background daemon thread when
GENKIT_ENV=devis set, right atGenkit()construction. The thread owns its ownasyncioevent loop so it never conflicts with FastAPI's, Flask's, or any other framework's loop. The registry is already protected by athreading.RLockso concurrent access from the background thread is safe.In the case of a web app like FastAPI - Since both the Reflection Server and FastAPI have the ability to call a plugin from their own eventloop, there are model plugin bugs because the underlying model client errors out when executing from a different eventloop than the one that it was created in. Because of that, plugin authors need to create and cache a client on every running eventloop that executes the model client code.
We provide a convenience method to make that easier. Here's how Plugin code can be executed on either loop. Plugin authors need to write something like this now:
`
from genkit.core._loop_local import _loop_local_client
class MyPlugin(Plugin):
def init(self, **params):
self._client_params = params
self._runtime_client = _loop_local_client(
lambda: AsyncClient(**self._client_params)
)
... (reuse self._runtime_client() later to access the SDK client
`
Before:
Scripts needed
run_main():FastAPI apps needed an additional lifespan plugin — even for local development only:
After
FastAPI apps — no lifespan needed:
Future (Plugin Authors)
Re-export from loop-local client to genkit.plugin package (containing set of utilities and types commonly used by plugin authors)
Scripts — unchanged:
Changes
Genkit.__init__(): starts the reflection server in a background daemon thread whenGENKIT_ENV=devrun_main(): simplified — no longer starts the server itself, just runs the user coroutine then blocks on Ctrl+C to keep the daemon thread alivegenkit_lifespan(): deprecated to a no-op withDeprecationWarning— existing code keeps workingweb/manager/directory removed — the internal abstraction that existed to run multiple ASGI servers on the same event loop, which was the previous approach to this problemweb-multi-server,web-endpoints-hello,web-short-n-longsamples removedToolRequestLike,MatchableAction,ResourceFnProtocol,ASGIAppProtocolGenkitReflectionApiErrorWireFormat→ReflectionError,GenkitReflectionApiErrorWireFormat=>RefectionErrorDetails