Skip to content

null in an Enum crashes the generator #500

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
juspence opened this issue Sep 23, 2021 · 3 comments
Closed

null in an Enum crashes the generator #500

juspence opened this issue Sep 23, 2021 · 3 comments
Labels
🐞bug Something isn't working
Milestone

Comments

@juspence
Copy link
Contributor

juspence commented Sep 23, 2021

Describe the bug
Similar to #357
Client generation fails and gives a Python traceback if an enum like below is present in the generated OpenAPI schema.

NullEnum:
  enum:
  - null

To Reproduce
Steps to reproduce the behavior:

  1. Make a new OpenAPI Spec File matching the one below.
  2. Run "openapi-python-client generate --path your_spec_file.yaml"
  3. See a Python traceback about "AttributeError: 'NoneType' object has no attribute 'replace'"
  4. Change "null" in the OpenAPI spec to "anyothervalue".
  5. See that generating the API client now succeeds.
  6. Change "anyothervalue" back to "null".
  7. Change line 62 of parser/properties/enum_property.py like below? Not sure if this is the right behavior:
    output[sanitized_key] = utils.remove_string_escapes(value) if isinstance(value, str) else value
  8. See that generating the API client now succeeds.

Expected behavior
API client is generated successfully, or an error message is shown about "Failed to parse OpenAPI document" if this enum is invalid.

OpenAPI Spec File

openapi: 3.0.3
info:
  title: title
  version: 1.0.0
paths:
  /path/to/something/:
    get:
      responses:
        '200':
          description: ''
components:
  schemas:
    ValueEnum:
      enum:
      - value
    NullEnum:
      enum:
      - null

Desktop (please complete the following information):

  • OS: Linux 4.18
  • Python Version: 3.6.8
  • openapi-python-client version 0.10.4

Additional context
Traceback (most recent call last):
File "/usr/local/bin/openapi-python-client", line 11, in
sys.exit(app())
File "/usr/local/lib/python3.6/site-packages/typer/main.py", line 214, in call
return get_command(self)(*args, **kwargs)
File "/usr/local/lib/python3.6/site-packages/click/core.py", line 829, in call
return self.main(*args, **kwargs)
File "/usr/local/lib/python3.6/site-packages/click/core.py", line 782, in main
rv = self.invoke(ctx)
File "/usr/local/lib/python3.6/site-packages/click/core.py", line 1259, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/usr/local/lib/python3.6/site-packages/click/core.py", line 1066, in invoke
return ctx.invoke(self.callback, **ctx.params)
File "/usr/local/lib/python3.6/site-packages/click/core.py", line 610, in invoke
return callback(*args, **kwargs)
File "/usr/local/lib/python3.6/site-packages/typer/main.py", line 497, in wrapper
return callback(**use_params) # type: ignore
File "/usr/local/lib/python3.6/site-packages/openapi_python_client/cli.py", line 148, in generate
config=config,
File "/usr/local/lib/python3.6/site-packages/openapi_python_client/init.py", line 329, in create_new_client
config=config,
File "/usr/local/lib/python3.6/site-packages/openapi_python_client/init.py", line 296, in _get_project_for_url_or_path
openapi = GeneratorData.from_dict(data_dict, config=config)
File "/usr/local/lib/python3.6/site-packages/openapi_python_client/parser/openapi.py", line 446, in from_dict
schemas = build_schemas(components=openapi.components.schemas, schemas=schemas, config=config)
File "/usr/local/lib/python3.6/site-packages/openapi_python_client/parser/properties/init.py", line 655, in build_schemas
schemas_or_err = update_schemas_with_data(ref_path=ref_path, data=data, schemas=schemas, config=config)
File "/usr/local/lib/python3.6/site-packages/openapi_python_client/parser/properties/schemas.py", line 93, in update_schemas_with_data
data=data, name=ref_path, schemas=schemas, required=True, parent_name="", config=config
File "/usr/local/lib/python3.6/site-packages/openapi_python_client/parser/properties/init.py", line 627, in property_from_data
name=name, required=required, data=data, schemas=schemas, parent_name=parent_name, config=config
File "/usr/local/lib/python3.6/site-packages/openapi_python_client/parser/properties/init.py", line 532, in _property_from_data
config=config,
File "/usr/local/lib/python3.6/site-packages/openapi_python_client/parser/properties/init.py", line 323, in build_enum_property
values = EnumProperty.values_from_list(enum)
File "/usr/local/lib/python3.6/site-packages/openapi_python_client/parser/properties/enum_property.py", line 62, in values_from_list
output[sanitized_key] = utils.remove_string_escapes(value)
File "/usr/local/lib/python3.6/site-packages/openapi_python_client/utils.py", line 96, in remove_string_escapes
return value.replace('"', r""")

@juspence juspence added the 🐞bug Something isn't working label Sep 23, 2021
@dbanty
Copy link
Collaborator

dbanty commented Sep 25, 2021

I think the problem is that null is a special value in YAML correlating to Python's None which is not a valid value in an Enum at all. If you want the string "null", you'll need to put it in quotes in the YAML like this:

NullEnum:
  enum:
  - "null"

The fact that you're getting a traceback is definitely a bug, you should instead get a nicely formatted warning that None is not a valid value for a String Enum.

@dbanty dbanty changed the title Support null in enums null in an Enum crashes the generator Sep 25, 2021
@dbanty dbanty added this to the 0.10.5 milestone Sep 25, 2021
@dbanty
Copy link
Collaborator

dbanty commented Sep 26, 2021

I read around the JSONSchema spec, and it seems like it supports literally any value as an enum. However, I'm not sure how we'd go about doing that in a way that's type safe and idiomatic in Python.

I'm going to use this issue to fix the crash & traceback in 0.10.5 and tighten up the parser to fail quick in the event of an enum we don't support. However, if you need None to be a value in a generated enum, we could maybe do that in a separate issue? Would just take some thinking since we couldn't use the multiple inheritance from str trick we do now to make the enums more useful.

Assuming you do need None to be a possible value for an Enum, could you add thoughts on how you'd like the Python code to look / work in a new issue? Maybe we could just treat it like optional=True as a special case?

@dbanty dbanty closed this as completed in f5b5ce8 Sep 26, 2021
@juspence
Copy link
Contributor Author

@dbanty
We do need None / null to be handled since our existing API has this in the generated OpenAPI schema. But I'm not very familiar with OpenAPI, why this value appears, or which API endpoint it comes from. I'll do some research and get back to you with more details.

In the meantime, I was able to quickly work around this issue by changing line 62 of parser/properties/enum_property.py like below:
output[sanitized_key] = utils.remove_string_escapes(value) if isinstance(value, str) else value

Adding the "if isinstance" bit kept the generator from calling .replace() on a NoneType instead of a str. I don't know if putting None directly into output[sanitized_key] is the right behavior, though. The generation succeeded after that change, but I didn't test the generated client at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants