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

Disappearing Enums #63

Closed
jayvdb opened this issue May 22, 2020 · 8 comments
Closed

Disappearing Enums #63

jayvdb opened this issue May 22, 2020 · 8 comments

Comments

@jayvdb
Copy link
Contributor

jayvdb commented May 22, 2020

When I add another set of views which has a model that includes a status attribute, the previously defined StatusEnum is dropped and its values are moved inline to the pre-existing schemas where it was being referenced.

I can understand why that is happening, but it is a bit unusual to have components disappear when an extra unrelated API is added. These two APIs are separate - they are each only used wihin different tags. The name of the enum could be prefixed with the tag name so I had a OscarapiStatus and a SubscriptionStatus.

@tfranzel
Copy link
Owner

yes. that is undocumented behaviour. just to be on the same page. the reason this is happening is because you introduced a new serializer that had an enum with a taken name but a different set of options.

the disappearing is unfortunate, but it is not obvious what the right thing to do is here, except being defensive about it. going the "tag" prefix route would for example break all our enums for client generation. the solution would have to be a bit smarter.

@jayvdb
Copy link
Contributor Author

jayvdb commented May 22, 2020

In my approach, the tag would only be added if there are two enums with different values that are isolated from each other into different tags as already defined by drf-spectacular. I cant see how that would break any additional existing installation, as that same scenario is when the enums disappear, which is also a breakage for the client generated. Also worth noting this will typically only occur when someone installs a new DRF app into urlconf, so very likely the client code is going to need an regen, and the breakage in the client is only renaming a class - a disappearing class is much harder to adjust client code to fit the revised generated API.

@tfranzel
Copy link
Owner

i agree with everything you say except that the tag prefix is not a robust discriminator. it might be in some cases, but not everywhere. i think its easy to construct a case where you have collision under same tag again.

i'm in favor of adressing this issue. i like the prefixing idea, though i'm afraid the tag as prefix is not the right choice.

@jayvdb
Copy link
Contributor Author

jayvdb commented May 22, 2020

A collision under the same tag is within a single app, and currently already doesnt ever result in an Enum being generated, so that is a non-bug.

Cross-linking with #43 wrt auto-populating the top level tags with the tags that drf-spectacular is already injecting into the endpoint metadata.

@tfranzel
Copy link
Owner

A collision under the same tag is within a single app, and currently already doesnt ever result in an Enum being generated, so that is a non-bug.

yes it is an definitely an improvement and would reduce issue's scope. i'm just saying that it does not completely solve the core issue. if someone (including us) uses the tags differently than app scoping, you have not gained very much here.

also what is supposed to happen if someone uses multiple tags? just picking the first seems crude and "camelcase joining" them seems rather messy.

@jayvdb
Copy link
Contributor Author

jayvdb commented May 22, 2020

also what is supposed to happen if someone uses multiple tags?

To do that, they must be using @extend_schema, yes?

If so, they are also able to add other decorators/extensions in order to give the enums names of their own choosing.

We only need a default behaviour for the fully automatic schema generation that is safe/sane most of the time, so to speak. For most people, the cost/benefit of staying in fully automatic mode will weight heavily in the favour of leaving it alone, and ignore/accept minor generated API version breakages like renames occasionally. But disappearing first class named entities feels like it is not acceptable, and some other auto-generation approach is needed, even if it isnt ideal.

In any case, I am referring to the namespacing that drf-spectacular is using for its own generated tags, irrespective of any tags the end-user project applies. Those namespaces typically map to disjoint/lightly-connected apps within Django, where a status field is more likely to be consistent within each namespace, and quite likely to be dissimilar across multiple namespaces.

If that separation doesnt help disambiguate two enums, numerical suffixes can be used -- my typescript code generator uses that approach as a fallback for multiple retrieve(get) endpoints. It isnt pretty, and even a bit brittle if the schema spec re-arranges them, but even in that worst case scenario it is easy to adjust the hand written code to reconnect to the right endpoints. And if that fails, the compiler is very likely to notice that the wrong endpoints are being used.

@tfranzel
Copy link
Owner

@jayvdb i improved the enum handling. there should be no more disappearing enums. collisions are resolved with static suffices. warnings are emitted on problems.

https://drf-spectacular.readthedocs.io/en/latest/faq.html#i-get-warnings-regarding-my-enum-or-my-enum-names-have-a-weird-suffix

let me know if there is a problem. you can close the issue if all works out.

jayvdb added a commit to jayvdb/drf-spectacular that referenced this issue Jun 1, 2020
@tfranzel
Copy link
Owner

tfranzel commented Jun 5, 2020

closing this in favor of #51

we reduced the amount of disappearing enums to a minimum. the remaining case is having enums that transform from XEnum Xca22Enum``. that is due to the collision handling and cannot be solved automatically. the algorithm has to do something when where is a collision introduced that was not there before. with the new mechanism there won't be any enums that flat out "disappear".

@tfranzel tfranzel closed this as completed Jun 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants