[RTC-409] Switch openapi generator by roznawsk · Pull Request #21 · fishjam-dev/python-server-sdk

roznawsk · 2023-12-13T12:17:04Z

Migrated from openapi-generator to openapi-python-client
Since the new generator uses ruff, switched from pylint to ruff for linting and from black for code formatting
Fixed the missing docs for ServerNotifications

Most changes include new models in _openapi_client and code formatting - the're probably not worth looking at.

Take a look at:

_room_api.py, _recording_api.py - minor changes related to the new api
pyproject.toml
poetry_scripts.py
_webhook_notifier.py

The tests fail, since there have been some updates in Jellyfish. The related changes will follow in the next PR.

roznawsk · 2023-12-14T20:02:06Z

jellyfish/_room_api.py

+    def _request(self, method, **kwargs):
+        resp = method.sync(client=self._client, **kwargs)
+        if isinstance(resp, Error):
+            raise RuntimeError(resp.errors)


@Rados13 Do you think it's ok to just throw a RuntimeError if a request should fail?
Before we had a couple more specific Exception types, such as UnauthorizedException, NotFoundException, BadRequestException

I think that a specific Exception would be better.

Rados13 · 2023-12-15T09:27:57Z

jellyfish/_room_api.py

+    def _request(self, method, **kwargs):
+        resp = method.sync(client=self._client, **kwargs)
+        if isinstance(resp, Error):
+            raise RuntimeError(resp.errors)


I think that a specific Exception would be better.

jellyfish/_room_api.py

jellyfish/_webhook_notifier.py

tests/test_notifier.py

tests/test_recording_api.py

Karolk99

"In all API classes, the _request method is repeated. I suggest creating a class with a @staticmethod request, or alternatively, create a class that inherits from it."

openapi_templates/str_enum.py.jinja

tests/test_recording_api.py

tests/test_notifier.py

jellyfish/api/_room_api.py

roznawsk · 2023-12-17T19:55:42Z

poetry_scripts.py

-import pdoc as p
-from pdoc import render
+
+# ruff: noqa: E501


Ignore line length in this file

Why do we need this?

Ok, I fixed the string that was too long.

roznawsk · 2023-12-17T19:57:23Z

pyproject.toml

+ignore = []
+
+[tool.ruff.extend-per-file-ignores]
+"jellyfish/_openapi_client/**" = ["E501"]


Ignore line length for autogenerated files - some comments are too long and cannot be automatically wrapped.

Rados13 · 2023-12-18T07:49:00Z

jellyfish/_webhook_notifier.py



-def receive_json(json: Dict) -> Union[server_messages]:
+def receive_json(json: Dict) -> Union[SERVER_MESSAGE_TYPES]:


When I generate docs locally I get warnings like this:

Warn: Error parsing type annotation typing.Union[ForwardRef('ServerMessageComponentCrashed'), ForwardRef('ServerMessageHlsPlayable'), ForwardRef('ServerMessageMetricsReport'), ForwardRef('ServerMessagePeerConnected'), ForwardRef('ServerMessagePeerCrashed'), ForwardRef('ServerMessagePeerDisconnected'), ForwardRef('ServerMessageRoomCrashed'), ForwardRef('ServerMessageRoomCreated'), ForwardRef('ServerMessageRoomDeleted')] for jellyfish.receive_json. Import of ServerMessageComponentCrashed failed: name 'ServerMessageComponentCrashed' is not defined

Should it look like this? Also maybe we should add action to check if there are any warnings during docs generation?

Ok, just changed the return type of receive_json to resolve this shenanigans.

If warning is generated the exit code is still 0, and there is no --warning-as-errors flag. We could capture the output and check if there are any warnings, or if there is any output at all (there shouldn't be any).

We probably could solve this by using env PYTHONWARNINGS=error as suggested by pdoc maintainer in this issue.

jellyfish/api/_room_api.py

Rados13 · 2023-12-18T08:02:43Z

poetry_scripts.py

-import pdoc as p
-from pdoc import render
+
+# ruff: noqa: E501


Why do we need this?

roznawsk added 2 commits December 12, 2023 14:55

Migrate to python-generator

6e32eb8

Fix ruff

e7e1727

roznawsk force-pushed the RTC-409-autogenerate-openapi branch 2 times, most recently from 375e31f to d077fce Compare December 14, 2023 14:05

Ruff

14051ea

roznawsk force-pushed the RTC-409-autogenerate-openapi branch from d077fce to 14051ea Compare December 14, 2023 19:14

Add enum template

a2defad

roznawsk force-pushed the RTC-409-autogenerate-openapi branch from 0ad32ac to a2defad Compare December 14, 2023 19:53

roznawsk requested review from Karolk99 and Rados13 December 14, 2023 19:54

roznawsk commented Dec 14, 2023

View reviewed changes

roznawsk marked this pull request as ready for review December 14, 2023 20:25

roznawsk mentioned this pull request Dec 14, 2023

Update hls_subscribe function #22

Closed

Fix ServerMessage docs

d135601

Rados13 assigned roznawsk Dec 15, 2023

Rados13 reviewed Dec 15, 2023

View reviewed changes

Karolk99 reviewed Dec 15, 2023

View reviewed changes

openapi_templates/str_enum.py.jinja Outdated Show resolved Hide resolved

tests/test_recording_api.py Outdated Show resolved Hide resolved

tests/test_notifier.py Outdated Show resolved Hide resolved

roznawsk commented Dec 15, 2023

View reviewed changes

jellyfish/api/_room_api.py Show resolved Hide resolved

Add custom http errors

6d351b9

roznawsk force-pushed the RTC-409-autogenerate-openapi branch from 6c338ff to 6d351b9 Compare December 15, 2023 15:23

roznawsk requested review from Karolk99 and Rados13 December 15, 2023 15:24

Karolk99 approved these changes Dec 15, 2023

View reviewed changes

roznawsk changed the title ~~Rtc 409 autogenerate openapi~~ [RTC-409] Switch openapi generator Dec 17, 2023

roznawsk commented Dec 17, 2023

View reviewed changes

Update comment

23125ee

Rados13 reviewed Dec 18, 2023

View reviewed changes

Fix docs warning

14c0ea5

roznawsk requested a review from Rados13 December 18, 2023 18:04

Rados13 approved these changes Dec 19, 2023

View reviewed changes

Default line length of 88

fbbef8e

roznawsk merged commit f2f6e28 into main Dec 19, 2023

roznawsk deleted the RTC-409-autogenerate-openapi branch December 19, 2023 15:41



		def receive_json(json: Dict) -> Union[server_messages]:
		def receive_json(json: Dict) -> Union[SERVER_MESSAGE_TYPES]:

Conversation

roznawsk commented Dec 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Karolk99 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

roznawsk commented Dec 13, 2023 •

edited

Loading