Enum support for OpenAPI and Looker spec enum types #241
Conversation
- enum detection is written, need to verify tests w/ canary upgrade
Still lots to do, things are definitely broken for the generators right now
Lots of linty freshness Much more to go
- replacing `x-looker-values` with `enum` in converted OA spec - putting license statement into generated files using the `LICENSE` file contents in the package folder - made `commentHeader` a little smarter
- All languages have enum types now - Kotlin, Typescript, and Swift are not broken with this enum implementation, but enum-specific tests need to be written - Python isn't happy. More investigation required. Do not ship any SDK using the code generated by this branch!
- merged master that has api-explorer package - yarn build works - tests work in intellij - testUtils is not in package builds
The generated C# code is the best looking of all our generated code now. It optimizes method interfaces and has excellent documentation. It helps that C# has a clear documentation standard, but we can take advantage of some of the new methodologies it has for the other generators. Particularly Kotlin and Swift can be made much cleaner. Added unit test for C# enum support also. More method unit tests are still required, but Look# is moving along nicely. There's also an alias `c#` for `csharp` so both: ```sh yarn sdk csharp ``` and ```sh yarn sdk c# ``` work (language match is case-insensitive)
Also cleaned up apix's package.json a bit
jkaster
added
enhancement
There was a problem hiding this comment.
nice work! I have a few haphazard comments on the code-gen stuff (nothing blocking approval) but the python needs some tweaks to be functional. Once you have the python formatter fixed it would be good to have an additional integration/smoke test that verifies [de]serialization works in python for these new enums
| @@ -132,6 +135,7 @@ class ${this.packageName}(api_methods.APIMethods): | |||
| # ${this.warnEditing()} | |||
| import datetime | |||
| from typing import Any, MutableMapping, Optional, Sequence | |||
| import enum | |||
There was a problem hiding this comment.
let's sandwich this between datetime and typing - I don't think black alphabetizes imports
There was a problem hiding this comment.
I fixed this and pushed the change, but for some reason Github is refusing to update the status of this comment. 😖
There was a problem hiding this comment.
I also put
import attrfirst, but GitHub is absolutely refusing to show the code updates
- We need to turn off that sort-keys linting rule everywhere. It doesn't want to go away. - Cleaned up sdkModels tests based on PR feedback
- regenerated all SDK language source with right trimmed comment headers - A forwardref for enum types may still be required for Python?
There was a problem hiding this comment.
We synced offline but I started a draft PR (#242) that addresses the Python side of this PR and Joel will be continuing it in the meantime.
Counting on enums
This PR generates enums for all 5 language SDKs. If a parameter, property, or type has an
x-looker-valuesorenumvalue list declaration, a correspondingenumtype is created. Previously, parameters or properties that were usually typed asstringare now an enum type.Examples
C#
Kotlin
Python
Swift
Typescript
TODO - Python unit test
The Python SDK still needs an enum property serialization/deserialization test. All other SDKs have at least one working test.
BONUS: C# generator optimization
This PR also optimizes C# method generation so generic type specifiers are only required for responses that take more than one type.
or
will generate the C# SDK code. C# is also generated with all other languages if a language specifier isn't included.