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

python: Use openapi-codegen for high-level python API client #1646

Merged
merged 15 commits into from
Jan 20, 2025

Conversation

svix-mman
Copy link
Contributor

@svix-mman svix-mman commented Jan 16, 2025

The openapi.json file I am working with is incomplete.

  1. Some of the deprecated methods are not included in the openapi.json.
  2. Some of the API resources are missing, background_task and operational_webhook. (I know that operational_webhook is not a single resource)
  3. Some API resources that are in the openapi.json are not currently in the high-level API client (health and environment).

Jonas is working on a fix for the openapi.json generator.

For now.

  1. I will manually add these deprecated calls by hand. And add a @deprecated decorator to those methods.
  2. If the API resource is not included in the openapi.json, I will leave it as is.
  3. For API resources that are not currently in the high-level API client, but are included in the openapi.json, I will ignore them for now.

Part of https://github.com/svix/monorepo-private/issues/9675

@svix-mman svix-mman force-pushed the mendy/add-generated-python-client-code branch from 0a45271 to f724642 Compare January 16, 2025 20:24
@svix-mman
Copy link
Contributor Author

(I force-pushed a typo fix in the commit message)

@svix-mman svix-mman force-pushed the mendy/add-generated-python-client-code branch from f342fb5 to 6cb2f29 Compare January 16, 2025 20:43
@svix-mman svix-mman marked this pull request as ready for review January 16, 2025 21:29
@svix-mman svix-mman requested a review from a team as a code owner January 16, 2025 21:29
Base automatically changed from mendy/sort-python-methods-consistently to main January 17, 2025 12:13
Copy link
Member

@svix-jplatte svix-jplatte left a 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.

python/svix/api/endpoint.py Show resolved Hide resolved
python/svix/api/endpoint.py Show resolved Hide resolved
python/svix/api/event_type.py Show resolved Hide resolved
python/svix/api/event_type.py Show resolved Hide resolved
python/svix/api/message_attempt.py Show resolved Hide resolved
python/svix/api/statistics.py Show resolved Hide resolved
@svix-mman
Copy link
Contributor Author

svix-mman commented Jan 17, 2025

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 ChnageLog.md

Copy link
Member

@svix-jplatte svix-jplatte left a 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 😅

ChangeLog.md Outdated Show resolved Hide resolved
ChangeLog.md Outdated Show resolved Hide resolved
ChangeLog.md Outdated Show resolved Hide resolved
ChangeLog.md Outdated Show resolved Hide resolved
ChangeLog.md Outdated Show resolved Hide resolved
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
@svix-mman svix-mman force-pushed the mendy/add-generated-python-client-code branch from a2949fd to 0406384 Compare January 17, 2025 20:08
@svix-jplatte svix-jplatte merged commit 53e4c12 into main Jan 20, 2025
8 checks passed
@svix-jplatte svix-jplatte deleted the mendy/add-generated-python-client-code branch January 20, 2025 10:49
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

Successfully merging this pull request may close these issues.

3 participants