🐛 Fix non-deterministic openapi schema in case of schema name duplication#14409
🐛 Fix non-deterministic openapi schema in case of schema name duplication#14409mattalexanderhill wants to merge 2 commits intofastapi:masterfrom
Conversation
During field remapping stage of openapi schema generation the order of iteration is not consistent across runs. This leads to non-deterministic schema generation Preserve the order of this iteration across runs by sorting by the "new_name", i.e. the value inserted in this mapping
|
I converted it to Draft. Please mark as Ready for review when it's ready |
|
Thanks @YuriiMotov should be ready for review. I don't think I have perms to update the labels, I'll let you decide on most appropriate here |
There was a problem hiding this comment.
@mattalexanderhill, thanks for your interest and efforts!
I think we need to add a warning. Without warning it will hide the problem (if there is naming conflict we will just use last model, but if they have different set of fields, the final openapi schema will be invalid).
I think it should be something like:
old_name_to_new_name_map = {}
+ name_to_model_map = {}
for field_key, schema in sorted(
field_mapping.items(),
key=lambda x: model_name_map.get(x[0][0].type_, ""),
):
model = field_key[0].type_
if model not in model_name_map:
continue
new_name = model_name_map[model]
old_name = schema["$ref"].split("/")[-1]
if old_name in {f"{new_name}-Input", f"{new_name}-Output"}:
continue
+ if old_name in name_to_model_map and name_to_model_map[old_name] is not model:
+ prev_model = name_to_model_map[old_name]
+ warnings.warn(
+ (
+ f"Model name conflict detected for model {model.__name__}. "
+ f"Both {model.__module__}.{model.__name__} and "
+ f"{prev_model.__module__}.{prev_model.__name__} "
+ f"are mapped to the same schema name {old_name}. "
+ ),
+ category=UserWarning,
+ stacklevel=2,
+ )
old_name_to_new_name_map[old_name] = new_name
+ name_to_model_map[old_name] = model
...I would actually raise an error, not warning.
@mattalexanderhill, what do you think?
YuriiMotov
changed the title
|
Alternative solution: #14797 |
github-actions
Bot
added
the
conflicts
|
This pull request has a merge conflict that needs to be resolved. |
|
This seems to be resolved in latest versions of FastAPI - schema is deterministic again. Thanks! |
2 participants
See #14376
During field remapping stage of openapi schema generation the order of iteration is not consistent across runs. This leads to non-deterministic schema generation
Preserve the order of this iteration across runs by sorting by the "new_name", i.e. the value inserted in this mapping