fix(generators): serialize timeout and max_retries in OpenAIImageGenerator.to_dict - #12290
fix(generators): serialize timeout and max_retries in OpenAIImageGenerator.to_dict#12290Anai-Guo wants to merge 1 commit into
Conversation
|
@Anai-Guo is attempting to deploy a commit to the deepset Team on Vercel. A member of the Team first needs to authorize it. |
|
|
…rator.to_dict `OpenAIImageGenerator.__init__` accepts `timeout` and `max_retries` and `_client_kwargs()` uses both when building the OpenAI client, but `to_dict()` never emitted them. A pipeline that is serialized and loaded back therefore silently falls back to `OPENAI_TIMEOUT`/`OPENAI_MAX_RETRIES` or to the 30s / 5 retries defaults. Every other OpenAI-backed component already serializes them (`OpenAIChatGenerator`, `OpenAIGenerator`, the Azure chat generators, the OpenAI/Azure embedders, `OpenAITokenCounter`); this one was the only outlier. `response_format` stays out on purpose - it is documented as ignored and kept only for backward compatibility.
13b94c7 to
d62a307
Compare
|
The changes look good to me @Anai-Guo ! Thank you for opening this PR. As soon as you agree to the CLA we can merge this PR. |
|
Hi @Anai-Guo, thanks a lot for your contribution! 🙏 We noticed that the Contributor License Agreement (CLA) check ( To get your PR reviewed, please sign the CLA via the link in the |
|
Noting for the record that this is blocked only on the |
|
Hi @Anai-Guo, just a friendly reminder: this PR is still in draft because the Contributor License Agreement (CLA) hasn't been signed yet. We'd love to review your contribution! Please sign the CLA via the link in the |
|
Friendly ping @julian-risch — this is approved but currently shows as BLOCKED by branch protection. Is there anything else needed from my side before it can be merged? |
|
Hi @Anai-Guo, just a friendly reminder: this PR is still pending because the Contributor License Agreement (CLA) hasn't been signed yet. We'd love to merge your contribution! Please sign the CLA via the link in the |
|
Hi @Anai-Guo, this PR is still waiting for the Contributor License Agreement (CLA) to be signed. Please note that if the CLA isn't signed within the next 4 days, this PR will be automatically closed as stale. Signing only takes a minute via the link in the |
Related Issues
to_dict()coverage across components.Proposed Changes:
OpenAIImageGenerator.__init__takestimeoutandmax_retries, stores both on the instance, and_client_kwargs()passes them toOpenAI/AsyncOpenAI.to_dict()never emitted them, so they were lost on the way through a serialized pipeline:The fallback is silent:
_client_kwargs()readsOPENAI_TIMEOUT/OPENAI_MAX_RETRIESor defaults to 30s / 5 retries whenever the attribute isNone, so a deserialized pipeline just runs with different client settings than the one that was saved.This is the only OpenAI-backed component with the gap —
OpenAIChatGenerator,OpenAIGenerator,AzureOpenAIChatGenerator,AzureOpenAIResponsesChatGenerator,OpenAIResponsesChatGenerator,OpenAI{Text,Document}Embedder,AzureOpenAI{Text,Document}EmbedderandOpenAITokenCounterall serialize both fields already.response_formatis deliberately left out: it is documented as ignored and only kept for backward compatibility, and__init__warns when it is set to anything other than"b64_json".How did you test it?
hatch run test:unit -k openai_image_generatorequivalent (pytest test/components/generators/test_openai_image_generator.py):3 failed, 20 passed, 2 skipped— the two updatedto_dictexpectations plus the new round-trip test23 passed, 2 skippedUpdated
test_to_dict,test_to_dict_with_paramsandtest_from_dictto cover the two fields (test_to_dict_with_paramsalready passedtimeout=60, max_retries=10into the constructor but asserted they were dropped), and addedtest_to_dict_from_dict_roundtrip_keeps_client_kwargs, which asserts_client_kwargs()is identical across ato_dict()/from_dict()round trip.ruff checkandruff format --checkare clean on both touched files.Notes for the reviewer
The branch is cut from an older
mainbecause my fork cannot be fast-forwarded (my token has noworkflowscope and syncing touches.github/workflows/). The three hunks are pure additions and merge cleanly onto currentmain; happy to rebase if you prefer.Checklist
This PR was written with an AI assistant (Claude Code). I have reviewed the changes and run the relevant tests locally.
🤖 Generated with Claude Code