NeuroVault / neurovault-python-sdk

automatically generated SDK code
1 stars 0 forks source link

Null enum defined incorrectly #1

Closed spacether closed 1 year ago

spacether commented 2 years ago

@jdkent Looking at https://github.com/NeuroVault/neurovault-python-sdk/blob/a089b149b364d6c5b3fb662854104173a87ad74f/docs/models/NullEnum.md it only allows string in but has an enum value of None. You need to add nullable: true to that schema to have the None value pass type checking

jdkent commented 2 years ago

potential related issue: https://github.com/tfranzel/drf-spectacular/issues/235

Hi, thanks for bringing this to my attention, we used drf-spectacular to generate the openapi schema.

Here is the openapi-generator command we used

    docker run --rm --env JAVA_OPTS="${JAVA_OPTS} -Dlog.level=debug" --user $(id -u):$(id -g) \
        -v $PWD:/local openapitools/openapi-generator-cli:v6.2.0 generate \
        -i /local/NeuroVault/openapi/openapi-schema.yml \
        -g python \
        -o /local/python/neurovault-python-sdk \
        -c /local/neurovault_python_sdk_config.json

NullEnum appears to be typeless in the schema so does that mean openapi-generator is giving a default type of string?

regardless, I may be able to bypass this by setting ENUM_ADD_EXPLICIT_BLANK_NULL_CHOICE: False in spectacular settings for the django project. Does that sound like a reasonable solution?

spacether commented 2 years ago

Per https://github.com/NeuroVault/neurovault-python-sdk/blob/main/neurovault_sdk/model/null_enum.py#L28 It looks like swagger-parser sets this as a type string schema even though type was omitted in the spec. So it is a swagger-parser bug probably. How about updating your schema to be type string with nullable: true? Or just type null?

spacether commented 2 years ago

Using the ENUM_ADD_EXPLICIT_BLANK_NULL_CHOICE also looks like a fine option.

jdkent commented 2 years ago

Per https://github.com/NeuroVault/neurovault-python-sdk/blob/main/neurovault_sdk/model/null_enum.py#L28 It looks like swagger-parser sets this as a type string schema even though type was omitted in the spec. How about updating your schema to be type string with nullable: true? Or just type null?

For this project, I'm trying to do minimal postprocessing of the schema and rely on third party tools to generate the schema and the subsequent SDK (less for me to maintain/break accidently)

Using the ENUM_ADD_EXPLICIT_BLANK_NULL_CHOICE also looks like a fine option.

:rocket: :rocket: I'll go with this option to see if it fixes the issue

spacether commented 2 years ago

Sounds good. FYI, this may work as is in the future when https://github.com/swagger-api/swagger-parser/issues/1792 is fixed