Skip to content

Name clashes between auto-generated models and explicitly defined models #323

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
forest-benchling opened this issue Feb 1, 2021 · 3 comments
Labels
🐞bug Something isn't working

Comments

@forest-benchling
Copy link
Collaborator

Describe the bug
We have a model Entity with a property schema:

Entity:
      type: object
      additionalProperties: false
      properties:
        schema:
          allOf:
            - "$ref": "#/components/schemas/SchemaSummary"
          nullable: true

and we also have a model EntitySchema.

The auto-generated client has a model EntitySchema, which refers to the property on Entity, but it silently ignores the explicitly defined EntitySchema model.

To Reproduce
Generate the client with Entity and EntitySchema as above.

Expected behavior
There should be two separate models, one for the Entity.schema property, one for EntitySchema.

OpenAPI Spec File
See above

Desktop (please complete the following information):

  • OS: 10.15.4
  • Python Version: 3.8.5
  • openapi-python-client version v0.7.3
@forest-benchling forest-benchling added the 🐞bug Something isn't working label Feb 1, 2021
@forest-benchling
Copy link
Collaborator Author

cc @bowenwr @dtkav, I see that we have worked around this for now by naming it TagSchema and BatchEntitySchema, but not ideal in the long run

@dbanty
Copy link
Collaborator

dbanty commented Feb 1, 2021

I believe you can use the title attribute to effectively choose a class name instead of relying on auto-generation. Not sure how that works with properties, but the reason I insist on keeping that feature despite some arguments to the contrary is precisely to give a tool around naming collisions.

That being said, we certainly shouldn't be skipping it silently, there should be some sort of an error that we generated the name twice. Also, we probably want the ability to override class names in config to be able to point at specific components rather than based on generated class name, so you can solve the collision without changing the OpenAPI document.

This is somewhat related to #300 which is trying to deal with the issue of component resolution vs class name deduplication. Fixing that wouldn't fix this, but if you are looking to write a solution involving the Schemas.models in the parser you probably should read that PR first.

@dbanty
Copy link
Collaborator

dbanty commented Apr 4, 2021

I think that this is as resolved as it can be with #336 being done, so I'm going to close it.

@dbanty dbanty closed this as completed Apr 4, 2021
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