Remove pydantic warnings when using model_dump as the default output is then overridden (#17939)

[BaptisteSaves]

Fixes #17939

Testes locally that the Pydantic warnings are not appearing anymore.

PR checklist

• Read the contribution guidelines.
• Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• Run the following to build the project and update samples:

  ./mvnw clean package 
  ./bin/generate-samples.sh ./bin/configs/*.yaml
  ./bin/utils/export_docs_generators.sh
  

  (For Windows users, please run the script in Git BASH)
  Commit all changed files.
  This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
  These must match the expectations made by your contribution.
  You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
  IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
• File the PR against the correct branch: master (upcoming 7.1.0 minor release - breaking changes with fallbacks), 8.0.x (breaking changes without fallbacks)
• If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.
  cc @cbornet @tomplus @krjakbrjak @fa0311 @multani

[BaptisteSaves]

Can you help me on this PR 🙏
The change is really simple and should not impact anything.

It says some tests are failing but without giving me which one, and everything is passing fine locally.
Also I am on linux so I cannot test on windows.

[wing328]

I've restarted the failed jobs so let's see how that goes

[multani]

@BaptisteSaves : are you sure this still happens with 7.3.0?

In #17511 I changed things related to how Literal were used and this should have fixed issues like the one you mention in #17939.

[BaptisteSaves]

@multani Yes I am sure, I tested on 7.2, 7.3 and latest master jar.

If you cannot reproduce with the OpenApi version I gave maybe It could be linked to specific version of Python and Pydantic V2 ?

[BaptisteSaves]

$docker run --rm -v "${PWD}:/local" -u 1000:1000  openapitools/openapi-generator-cli:v7.3.0 generate          -i /local/openapi.yml   -g python -o /local/client
…
# Thanks for using OpenAPI Generator.                                          #
$ cd client/
$ python

Python 3.10.13 (main, Sep 11 2023, 06:05:40) [GCC 13.2.1 20230801] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from openapi_client import ObjectDto
>>> from openapi_client import ObjectDtoPropertyA
>>> ObjectDto(property_a=ObjectDtoPropertyA("test")).to_dict()
XXX/venv/lib/python3.10/site-packages/pydantic/main.py:308: UserWarning: Pydantic serializer warnings:
  Expected `list[str]` but got `_LiteralGenericAlias` - serialized value may not be as expected
  return self.__pydantic_serializer__.to_python(
{'propertyA': 'test'}

[multani]

@BaptisteSaves Thanks, for the confirmation 👍

I also checked on my side and saw the issue as well.

I think there could be a better fix than ignoring all the warnings (we adjusted lot of mypy/Pydantic settings in the past couple of months, there are probably still some problems) but I won't have time to look at it further this week.

[BaptisteSaves]

@multani

I checked the PR you linked and we can see this change in the AnyOf template

    any_of_schemas: List[str] = Literal[{{#lambda.uppercase}}{{{classname}}}{{/lambda.uppercase}}_ANY_OF_SCHEMAS]
    any_of_schemas: List[str] = Field(default=Literal[{{#anyOf}}"{{.}}"{{^-last}}, {{/-last}}{{/anyOf}}])

Testing locally the warning does not appear if I remove Literal[]

    any_of_schemas: List[str] = Field(default=[{{#anyOf}}"{{.}}"{{^-last}}, {{/-last}}{{/anyOf}}])

I am lost on the impact of removing Literal here, I could change the PR to remove it if you think that makes sense

Also, could you explain the goal of this attribute any_of_schemas please 🙏 It does not seem to be used anywhere nor be part of Pydantic or Python typing

[multani]

@BaptisteSaves initially, using Pydantic v1, this was written as:

any_of_schemas: List[str] = Field(xxx, const=True)

In #16624, this was changed to:

any_of_schemas: List[str] = Literal[xxx]

because Pydantic was returning:

pydantic.errors.PydanticUserError: `const` is removed, use `Literal` instead

For further information visit https://errors.pydantic.dev/2.6/u/removed-kwargs

In #17511, this was changed to:

any_of_schemas: List[str] = Field(default=Literal[xxx])

which is probably not super correct either.

Retrospectively, we can remove Literal as you proposed, and that should be doing the same, although typing-ly speaking, it won't be marked as "constant" anymore as the initial implementation.

Propositions

@wing328 I'm also not super sure what's the purpose of any_of_schemas or the "constant" XXX_ANY_OF_SCHEMAS (and the related one_of_schema).

I assumed this was some kind of "metadata" to give the indication which "schema" the class supports. Do you have more information about these fields and their purpose?

Fixing "one of" fields

For "one of" field (for instance in the Pig model), I guess it would make sense to have:

one_of_schema: Literal["X", "Y"]

... since I suppose the goal was to expose which model the underlying value is.

The problem is that this field doesn't have a default value and is not set anywhere in the code. @wing328 is this a bug (the field should be set with a specific value and it doesn't) or is there another meaning behind this "one of" field?

Fixing "any of" fields

For "any of" field (for instance in the AnyOfColor model), it's not clear what a proper value would be (again, because it's not clear what's the intent of the field). The field is also never set, so it's the same problem as the "one of" field above.

@wing328 would it make sense to replace:

any_of_schemas: List[str] = Field(default=Literal["X", Y"])

by:

any_of_schemas: List[str] = Field(default=["X", Y"])

The previous version is incorrect anyway (List[str] and Literal["X", "Y"] are actually not compatible, AFAIU).

[wing328]

I assumed this was some kind of "metadata" to give the indication which "schema" the class supports.

yes

would it make sense to replace:

please go ahead as I don't have a strong opinion on this.

[multani]

Thanks @wing328

would it make sense to replace:

please go ahead as I don't have a strong opinion on this.

Maybe not for this change here, but is it really necessary? Is it something that could be removed?
I have no idea how it is used at the moment, but it's untested and barely documented.

[fa0311]

would it make sense to replace:

At least any_of_schemas is not a single value, so it should not be a literal type.

If literals are used correctly, it would look like this
However, I am not sure if it makes sense to explicitly specify literals.

any_of_schemas: List[Literal["X", Y"]] = Field(default=["X", Y"])

Alternatively, you can use Set

any_of_schemas: Set[str] = Set({"xxx", yyy"})

[BaptisteSaves]

@fa0311 Yes but we already have a constant with this value

openapi-generator/samples/openapi3/client/petstore/python-aiohttp/petstore_api/models/any_of_color.py

Line 27 in 5d43c88

ANYOFCOLOR_ANY_OF_SCHEMAS = ["List[int]", "str"]

So maybe we could simply remove this any_of_schemas ?

[multani]

@BaptisteSaves I would say for now:

• Fix the definitions of *_of_schemas as proposed above so that the warnings are gone but we don't completely remove the warnings from model_dump()
• We can create a follow-up issue to discuss what to do with the constants and the actual fields 👍

This way we can merge your change and remove the warnings and we can discuss afterwards how to improve these fields/constants .

[BaptisteSaves]

@fa0311 @multani PR updated to just change one_of and any_of from Field to a simple set

[multani]

lgtm, thanks @BaptisteSaves !

[fa0311]

LGTM👍

[BaptisteSaves]

@wing328 Can you merge the PR 🙏 ?

[wing328]

just merged. thanks for the PR

[samertm]

Looking forward to the release with this fix! We currently get lots of "Expected list[str] but got _LiteralGenericAlias - serialized value may not be as expected" errors in our logs. Do you know when the next release is going to go out?

[wing328]

https://github.com/OpenAPITools/openapi-generator/milestones

but may be delayed as we may have a few more changes we want to ship as part of the 7.5.0 release

you may consider using the snapshot version for the time being

(snapshot version can be found in the project's readme)