Temporary workaround for generating unique model names #47
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
efriis
reviewed
Oct 16, 2023
| @@ -85,8 +107,17 @@ def _resolve_model(type_: Union[Type, BaseModel], default_name: str) -> Type[Bas | |||
|
|
|||
| hash_ = model.schema_json() | |||
|
|
|||
| if model.__name__ in _SEEN_NAMES and hash_ not in _MODEL_REGISTRY: | |||
| # If the model name has been seen before, but the model itself is different | |||
There was a problem hiding this comment.
is the order models get loaded in deterministic? Could this result in an indeterministic openapi spec depending on that order?
There was a problem hiding this comment.
Depends on order of add_routes invocation so I think deterministic in this case. [This is because we're only registering top level model names (which is also why we can't resolve the full issue)]
langserve/server.py
Outdated
| @@ -192,18 +208,22 @@ def add_routes( | |||
| "Use `pip install sse_starlette` to install." | |||
| ) | |||
|
|
|||
| namespace = path or "" | |||
|
|
|||
| model_namespace = path.strip("/").replace("/", "_") | |||
There was a problem hiding this comment.
Are there restrictions on pydantic names?
- if so, there are more characters in paths that we might need to handle
- if not, can we skip this and pass around the
pathinstead? Might be more straightforward than a newnamespaceconcept
There was a problem hiding this comment.
Good points. Will check wasn't being methodlogical here
Co-authored-by: Erick Friis <[email protected]>
eyurtsev
changed the title
efriis
approved these changes
Oct 17, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If two schema different schemas share the same name, fastapi openapi documentation fails.
This is a temporary work-around for openapi spec generation purposes. It will work only for top level model names.
We've bumped into this issue while hosting 2 runnables that start with a PromptTemplate (same name for the schema), but have different template variables (so the schema are different).
We'll need to find a better solution to handle nested models. This should be good for now as it'll allow users to deploy the common case correctly.