-
Notifications
You must be signed in to change notification settings - Fork 178
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
python: Use openapi-codegen for high-level python API client #1646
Conversation
0a45271
to
f724642
Compare
(I force-pushed a typo fix in the commit message) |
f342fb5
to
6cb2f29
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good! I think we just need to document the breaking changes, then we can merge.
I will go through the diffs one at a time and document the breaking changes for the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have lots of thoughts on these changelog entries 😅
Note: the openapi.json file I am working with does not include some deprecated methods. For now I will manually add these deprecated calls by hand
Breaking change: in the create and update function the integ_(in/update) is renamed to integration_(in/update) This is due to the way that the codegen decides how to name function arg. (The type of the function arg to_snake_case) I don't think it makes sense to add a special case in the codegen to shorten `integration` to `integ` This problem is a bit more annoying, since the path param is called `integ_id` (in the `openapi.json`). So to keep consistency we may want to add the special case to the codegen Alternatively (and this would make me happy) we could change the path param name from `integ_id` to `integration_id`. (in the `openapi.json`) This will make the file consistent, and won't require a special case in the openapi-codegen
Braking changes: - `MessageAttemptListOptions` is not importable from `svix.api`
Manual edits: - For `v1.message.create` manually set `ret.payload` to `message_in.payload` - For `MessageCreateOptions` set the `with_content` false by default. (this is preserve previous behavior)
This also switches the `MessageListOptions` import from `message_attempt` to `message`, and removes the definition in `message_attempt` since it is not used by anything (except the `from svix.api` import, which we replaced) Breaking change: - The `status_code_class` and `status` optional fields are removed from `svix.api.MessageListOptions`
Technically? breaking change: - the `task_id` argument is removed from the `aggregate_event_types` method. However this argument was not previously used by the inner API client at all. So this technically breaks the high-level API client, but no change to the actual outgoing api.svix.com requests
a2949fd
to
0406384
Compare
The
openapi.json
file I am working with is incomplete.openapi.json
.background_task
andoperational_webhook
. (I know thatoperational_webhook
is not a single resource)openapi.json
are not currently in the high-level API client (health
andenvironment
).Jonas is working on a fix for the
openapi.json
generator.For now.
@deprecated
decorator to those methods.openapi.json
, I will leave it as is.openapi.json
, I will ignore them for now.Part of https://github.com/svix/monorepo-private/issues/9675