Skip to content
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

Conflicting module names for models silently generate invalid code #1163

Open
linkdd opened this issue Nov 11, 2024 · 4 comments
Open

Conflicting module names for models silently generate invalid code #1163

linkdd opened this issue Nov 11, 2024 · 4 comments
Labels
🐞bug Something isn't working

Comments

@linkdd
Copy link

linkdd commented Nov 11, 2024

Describe the bug

I am generating a Python Client using the OpenAPI model of Netbox 3.6.9 (with a few plugins pre-installed).
This schema contains for example a model VLAN and a model Vlan.

However, in the generated client, only the model Vlan is generated, but the models/__init__.py still tries to import VLAN.

OpenAPI Spec File

https://gist.github.com/linkdd/3193453b38357d190545c7988cdac692

Desktop:

  • OS: Debian 12
  • Python Version: 3.12
  • openapi-python-client version 0.21.6

Additional context

At $day_job, it was a blocking point, we had to remove temporarily the dependency to the generated client. I added a few tasks to my backlog in order to tackle this issue myself and ideally come up with a PR, but it won't be soon. Any help or information on how/where the model generation actually happens would be awesome and a huge time saver.

@dbanty
Copy link
Collaborator

dbanty commented Nov 12, 2024

Oh, interesting bug you've found! What's happening is that the module name selected for Vlan and VLAN is, for both vlan. So vlan.py is being generated twice, overwriting itself 😬. The fix is going to be to catch this issue and either warn about it or automatically rename the conflicting modules somehow.

However, there's a workaround for now, before I get to the fix (since I'm swamped this month). Because the generated class names are different, you can target them separately using a config file and override the module name. So something like this should work:

class_overrides:
  VLAN:  # Alternatively, select Vlan if you'd rather
    module_name: custom_vlan  # name this whatever you want

That should cause the second vlan.py to be named whatever you put in module_name instead, and the imports should be updated appropriately. You then invoke the generator like openapi-python-client generate --config path-to-config.yaml <other args here>. You could also change one or both of the class names there, if you like, by putting a class_name: where module_name: is.

@dbanty dbanty added the 🐞bug Something isn't working label Nov 12, 2024
@dbanty dbanty changed the title Model generation seems to be case-sensitive: missing model where only the case changes Conflicting module names for models silently generate invalid code Nov 12, 2024
@linkdd
Copy link
Author

linkdd commented Nov 12, 2024

The fix is going to be to catch this issue and either warn about it or automatically rename the conflicting modules somehow.

The openapi-generator tool name the second module vlan0.py (but it's written in Java, and the Python generator does not support async). I think it's a sane behavior I could try to mimic.

On how to catch it, I guess we could have a set of module names already used, and if the name is already in it we add a suffix. Or something like that. I'll try to look into it as soon as I have some time.

I'm swamped this month

No worries, there is no urgency at the moment :) Thanks for the work on this project.

@harikesavapk
Copy link

I'm facing the same issue. I have two different Enums, SIPStatus and SipStatus, but they have overwritten and are still trying to import from the same file:

from .sip_status import SIPStatus, SipStatus

@linkdd
Copy link
Author

linkdd commented Nov 19, 2024

I made a dirty change in a PR which solves the issue on my side. It relies on a global variable which is a code smell, but I'm not familiar enough with the codebase yet to know where is the proper location for this information (the Config object maybe? A contextvar? To be discussed...).

I'm still moving in my new house and assembling furniture, so I don't have a lot of free time, but still, that should be a good starting point towards a solution, and a temporary workaround for people encountering the same problem.

Let's move the discussion on this PR, I'll be happy to receive feedback and guidance from you @dbanty so that we can move forward 🙂

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

3 participants