Skip to content

Szedan rh/293 factoryfunc json decoder - #295

Open
szedan-rh wants to merge 2 commits into
llm-d:mainfrom
szedan-rh:szedan-rh/293-factoryfunc-json-decoder
Open

Szedan rh/293 factoryfunc json decoder#295
szedan-rh wants to merge 2 commits into
llm-d:mainfrom
szedan-rh:szedan-rh/293-factoryfunc-json-decoder

Conversation

@szedan-rh

Copy link
Copy Markdown
Contributor

/kind cleanup

Align FactoryFunc signature with EPP: json.RawMessage → *json.Decoder

Replace json.RawMessage with *json.Decoder in FactoryFunc and all 16
factory implementations. Config loader now wraps parameters via
StrictDecoder (imported from EPP) to enable DisallowUnknownFields
validation on plugin config parsing.

Closes #293

  Store plugin-shared data directly on InferenceRequest via a sync.Map
  attribute store, matching EPP's pattern. Pass request to response plugin
  signatures so they can read attributes set during request phase.
  Delete cycle_state.go entirely.

Signed-off-by: szedan <szedan@redhat.com>
  Replace json.RawMessage with *json.Decoder in FactoryFunc and all 16
  factory implementations. Config loader now wraps parameters via
  StrictDecoder (imported from EPP) to enable DisallowUnknownFields
  validation on plugin config parsing.

  Closes llm-d#293

Signed-off-by: szedan <szedan@redhat.com>
@github-actions github-actions Bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. labels Aug 12, 2026

@ArshVermaGit ArshVermaGit left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Small cleanup change aligning FactoryFunc with the EPP json.Decoder interface. The change is focused and looks good to me.

@nirrozenbaum nirrozenbaum left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@szedan-rh the PR mixes the removal of CycleState with the replacement of json decode function.
please align the changes with the purpose of the PR (update json decode)

@szedan-rh

szedan-rh commented Aug 30, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for flagging this @nirrozenbaum you're right, and here's why it happened: this branch (szedan-rh/293-factoryfunc-json-decoder) was stacked on top of szedan-rh/288-request-attributes-remove-cyclestate (#294), so the diff against main is picking up both the CycleState removal from #294 and the FactoryFunc/json.Decoder change that's actually in scope here.

Plan to fix: once #294 merges, I'll rebase this branch onto the updated main, which will drop the CycleState-related commits from this diff and leave only the json.RawMessage → *json.Decoder change that closes #293. Will ping this thread once that's done so it's clean for re-review.

@szedan-rh
szedan-rh requested a review from nirrozenbaum August 30, 2026 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(framework): align FactoryFunc signature with EPP (json.RawMessage → *json.Decoder)

3 participants