[data] Fix type hint for config parameter in build_processor#64098
[data] Fix type hint for config parameter in build_processor#64098ArchishmanSengupta wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the type hint of the config parameter in the public build_processor function from ProcessorConfig to the internal _ProcessorConfig. The reviewer advises against exposing internal classes in the public API and suggests keeping the public ProcessorConfig type hint by utilizing multiple inheritance on the configuration classes.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| @PublicAPI(stability="beta") | ||
| def build_processor( | ||
| config: ProcessorConfig, | ||
| config: _ProcessorConfig, |
There was a problem hiding this comment.
Exposing the internal/private class _ProcessorConfig in the public API signature of build_processor is not ideal, as it leaks internal implementation details to users and static analysis tools.
Instead of changing the type hint of build_processor to _ProcessorConfig, a cleaner and more idiomatic approach is to make the public config classes inherit from the public ProcessorConfig class as well. Since they already inherit from their respective internal classes (which inherit from _ProcessorConfig), adding ProcessorConfig as a secondary base class forms a valid diamond inheritance and correctly establishes them as subclasses of ProcessorConfig in the type system.
For example:
class HttpRequestProcessorConfig(_HttpRequestProcessorConfig, ProcessorConfig):
pass
class vLLMEngineProcessorConfig(_vLLMEngineProcessorConfig, ProcessorConfig):
passThis allows build_processor to keep its clean, public type hint.
| config: _ProcessorConfig, | |
| config: ProcessorConfig, |
1ffe119 to
5054291
Compare
Description
The public engine config classes (
vLLMEngineProcessorConfig,SGLangEngineProcessorConfig,HttpRequestProcessorConfig,ServeDeploymentProcessorConfig) only inherited from theirinternal base classes, not from the public
ProcessorConfigwrapper. This made them siblingtypes of
ProcessorConfigin the static type system, so type checkers (pyright, mypy, IDElinters) flagged valid calls like
build_processor(config=vLLMEngineProcessorConfig(...))astype errors, even though they work correctly at runtime.
This adds
ProcessorConfigas a secondary base class to each public config class. Since theyalready inherit from internal classes that derive from the same
_ProcessorConfigbase, thisforms a valid diamond and correctly establishes them as subclasses of the public
ProcessorConfig.build_processorkeeps its clean publicconfig: ProcessorConfigtype hint.No internal types are leaked into the public API and no runtime behavior changes.
Related issues
Fixes #57981
Additional information
Verified with pyright — the repro from the issue and all config types now produce 0 type errors.