openapi-python-client icon indicating copy to clipboard operation
openapi-python-client copied to clipboard

Conflicting module names for models silently generate invalid code

Open linkdd opened this issue 1 year ago • 4 comments

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.

linkdd avatar Nov 11 '24 12:11 linkdd

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 avatar Nov 12 '24 00:11 dbanty

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.

linkdd avatar Nov 12 '24 00:11 linkdd

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

harikesavapk avatar Nov 17 '24 06:11 harikesavapk

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 🙂

linkdd avatar Nov 19 '24 00:11 linkdd