search

openapi-generators / openapi-python-client

Generate modern Python clients from OpenAPI
MIT License
1.35k stars 206 forks source link

Conflicting module names for models silently generate invalid code #1163

Open linkdd opened 1 week ago

linkdd commented 1 week ago

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:

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 commented 1 week ago

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.

linkdd commented 1 week ago

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 commented 6 days ago

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 commented 4 days ago

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 🙂