Skip to content

Conversation

jonahjung22
Copy link
Collaborator

@jonahjung22 jonahjung22 commented Aug 28, 2025

Description:

Code changes:

  • UpdateModelParametersResponse was unused - frontend already has model_id, HTTP status codes handle errors
  • GetModelParametersResponse.count is unnecessary - frontend can get array length
  • ParameterSchema 'number' type was ambiguous vs 'integer' - now uses clear 'float' type
  • Missing Pydantic validation for GetModelParametersResponse and ParameterSchema

User facing changes:

  • No user-facing changes, internal API cleanup and improved type safety

@jonahjung22 jonahjung22 added the enhancement New feature or request label Aug 28, 2025
@jonahjung22 jonahjung22 self-assigned this Aug 28, 2025
@jonahjung22 jonahjung22 linked an issue Aug 28, 2025 that may be closed by this pull request
Copy link
Member

@dlqqq dlqqq left a comment

Choose a reason for hiding this comment

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

@jonahjung22 Thank you Jonah! I left a couple of minor suggestions below.

After addressing the feedback, please also re-test this PR now that it is rebased onto main with #1473 merged.

I recommend running git config --global pull.rebase true if you haven't already. After running that, running git pull should sync your local branch with this remote.

@dlqqq dlqqq force-pushed the simplify-model-param-restapi branch from 0832060 to 2629b63 Compare September 3, 2025 17:06
@dlqqq dlqqq changed the title Simplifying the model parameter rest api Simplify model parameter REST API Sep 3, 2025
Copy link
Member

@dlqqq dlqqq left a comment

Choose a reason for hiding this comment

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

The API looks great now, thank you for fixing it! 🎉

@dlqqq dlqqq merged commit c1250eb into jupyterlab:main Sep 4, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[model-parameters] Simplify the model parameters REST API
2 participants