Skip to content

fix(service): propagate text_keys to ops via get_init_configs in _set…#960

Merged
cmgzn merged 3 commits into
mainfrom
fix/service-text-key-propagation
Apr 8, 2026
Merged

fix(service): propagate text_keys to ops via get_init_configs in _set…#960
cmgzn merged 3 commits into
mainfrom
fix/service-text-key-propagation

Conversation

@cmgzn
Copy link
Copy Markdown
Collaborator

@cmgzn cmgzn commented Apr 3, 2026

…up_cfg

When cfg is constructed directly from a JSON string in the API path, it bypasses init_configs() and update_op_attr(), causing text_keys (e.g. 'output') to not be injected into each op's text_key parameter. This results in ops using the default text_key 'text', leading to 'NoneType' object is not subscriptable when the dataset field name differs from 'text'.

Fix: replace direct Namespace construction with get_init_configs() to ensure full config initialization including op attribute propagation.

…up_cfg

When cfg is constructed directly from a JSON string in the API path,
it bypasses init_configs() and update_op_attr(), causing text_keys
(e.g. 'output') to not be injected into each op's text_key parameter.
This results in ops using the default text_key 'text', leading to
'NoneType' object is not subscriptable when the dataset field name
differs from 'text'.

Fix: replace direct Namespace construction with get_init_configs() to
ensure full config initialization including op attribute propagation.
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the configuration initialization in service.py by replacing manual Namespace creation with the get_init_configs utility. The review identifies several critical issues with this implementation: it introduces side effects by reconfiguring the global logger on every request, contains a concurrency bug due to fixed temporary file paths in the underlying configuration logic, and fails to handle cases where the configuration is already parsed as a dictionary by FastAPI. A code suggestion was provided to address these issues by handling both data types and using load_configs_only=True.

Comment thread service.py Outdated
… race

- Call get_init_configs() in _setup_cfg to ensure text_keys is properly
  propagated to each operator's text_key parameter
- Use NamedTemporaryFile with delete=True to avoid race conditions when
  multiple API requests are processed concurrently
@cmgzn cmgzn merged commit 4698741 into main Apr 8, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants