Conversation
There was a problem hiding this comment.
Pull request overview
Adds MoQ (Media over QUIC) token generation support to the Python Fishjam client by extending the generated OpenAPI client and exposing new convenience methods on FishjamClient.
Changes:
- Introduces new OpenAPI client endpoints for creating MoQ publisher/subscriber tokens.
- Adds
FishjamClient.create_moq_publisher_tokenandFishjamClient.create_moq_subscriber_token. - Adds integration tests covering the new methods and unauthorized behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
fishjam/api/_fishjam_client.py |
Exposes new MoQ token methods on the public client API. |
fishjam/_openapi_client/api/moq/create_moq_publisher_token.py |
Generated OpenAPI endpoint wrapper for publisher token creation. |
fishjam/_openapi_client/api/moq/create_moq_subscriber_token.py |
Generated OpenAPI endpoint wrapper for subscriber token creation. |
fishjam/_openapi_client/api/moq/__init__.py |
Adds the moq API package. |
fishjam/_openapi_client/models/moq_token.py |
Adds the MoqToken response model. |
fishjam/_openapi_client/models/__init__.py |
Exports MoqToken from the OpenAPI models package. |
tests/test_room_api.py |
Adds tests for MoQ token generation and unauthorized errors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| create_moq_publisher_token, | ||
| create_moq_subscriber_token, |
There was a problem hiding this comment.
The OpenAPI endpoint modules are imported with the same names as the FishjamClient methods defined below (create_moq_publisher_token / create_moq_subscriber_token). This works due to Python name resolution, but it’s easy to misread and is inconsistent with the rest of this file where endpoint modules are aliased (e.g., room_create_room, viewer_generate_viewer_token). Consider aliasing these imports (e.g., moq_create_publisher_token, moq_create_subscriber_token) and using the aliases in _request(...) for clarity and to avoid accidental shadowing issues later.
| create_moq_publisher_token, | |
| create_moq_subscriber_token, | |
| create_moq_publisher_token as moq_create_publisher_token, | |
| create_moq_subscriber_token as moq_create_subscriber_token, |
| def create_moq_publisher_token(self, stream_name: str) -> str: | ||
| """Generates a MoQ publisher token for the given stream. | ||
|
|
||
| Args: | ||
| stream_name: The name of the MoQ stream. | ||
|
|
||
| Returns: | ||
| str: The generated publisher token. | ||
| """ | ||
| response = cast( | ||
| MoqToken, | ||
| self._request(create_moq_publisher_token, stream_id=stream_name), | ||
| ) | ||
|
|
||
| return response.token | ||
|
|
||
| def create_moq_subscriber_token(self, stream_name: str) -> str: | ||
| """Generates a MoQ subscriber token for the given stream. | ||
|
|
||
| Args: | ||
| stream_name: The name of the MoQ stream. | ||
|
|
||
| Returns: | ||
| str: The generated subscriber token. | ||
| """ | ||
| response = cast( | ||
| MoqToken, | ||
| self._request(create_moq_subscriber_token, stream_id=stream_name), |
There was a problem hiding this comment.
The method parameter is named stream_name, but the underlying endpoint path parameter is stream_id (and this method passes it as stream_id=...). To avoid confusing API consumers, consider renaming the argument to stream_id (or updating the docstring to clearly explain what identifier is expected) so the public API matches the underlying API semantics.
| def create_moq_publisher_token(self, stream_name: str) -> str: | |
| """Generates a MoQ publisher token for the given stream. | |
| Args: | |
| stream_name: The name of the MoQ stream. | |
| Returns: | |
| str: The generated publisher token. | |
| """ | |
| response = cast( | |
| MoqToken, | |
| self._request(create_moq_publisher_token, stream_id=stream_name), | |
| ) | |
| return response.token | |
| def create_moq_subscriber_token(self, stream_name: str) -> str: | |
| """Generates a MoQ subscriber token for the given stream. | |
| Args: | |
| stream_name: The name of the MoQ stream. | |
| Returns: | |
| str: The generated subscriber token. | |
| """ | |
| response = cast( | |
| MoqToken, | |
| self._request(create_moq_subscriber_token, stream_id=stream_name), | |
| def create_moq_publisher_token(self, stream_id: str) -> str: | |
| """Generates a MoQ publisher token for the given stream. | |
| Args: | |
| stream_id: The ID of the MoQ stream. | |
| Returns: | |
| str: The generated publisher token. | |
| """ | |
| response = cast( | |
| MoqToken, | |
| self._request(create_moq_publisher_token, stream_id=stream_id), | |
| ) | |
| return response.token | |
| def create_moq_subscriber_token(self, stream_id: str) -> str: | |
| """Generates a MoQ subscriber token for the given stream. | |
| Args: | |
| stream_id: The ID of the MoQ stream. | |
| Returns: | |
| str: The generated subscriber token. | |
| """ | |
| response = cast( | |
| MoqToken, | |
| self._request(create_moq_subscriber_token, stream_id=stream_id), |
| def create_moq_publisher_token(self, stream_name: str) -> str: | ||
| """Generates a MoQ publisher token for the given stream. | ||
|
|
||
| Args: | ||
| stream_name: The name of the MoQ stream. | ||
|
|
||
| Returns: | ||
| str: The generated publisher token. | ||
| """ | ||
| response = cast( | ||
| MoqToken, | ||
| self._request(create_moq_publisher_token, stream_id=stream_name), | ||
| ) | ||
|
|
||
| return response.token | ||
|
|
||
| def create_moq_subscriber_token(self, stream_name: str) -> str: | ||
| """Generates a MoQ subscriber token for the given stream. | ||
|
|
||
| Args: | ||
| stream_name: The name of the MoQ stream. | ||
|
|
||
| Returns: | ||
| str: The generated subscriber token. | ||
| """ | ||
| response = cast( | ||
| MoqToken, | ||
| self._request(create_moq_subscriber_token, stream_id=stream_name), |
There was a problem hiding this comment.
Same naming mismatch as above: the method argument is stream_name, but it’s forwarded as stream_id to the OpenAPI endpoint. Consider using stream_id in the public method signature (or clarifying in the docstring what identifier is expected) to keep the API consistent and unambiguous.
| def create_moq_publisher_token(self, stream_name: str) -> str: | |
| """Generates a MoQ publisher token for the given stream. | |
| Args: | |
| stream_name: The name of the MoQ stream. | |
| Returns: | |
| str: The generated publisher token. | |
| """ | |
| response = cast( | |
| MoqToken, | |
| self._request(create_moq_publisher_token, stream_id=stream_name), | |
| ) | |
| return response.token | |
| def create_moq_subscriber_token(self, stream_name: str) -> str: | |
| """Generates a MoQ subscriber token for the given stream. | |
| Args: | |
| stream_name: The name of the MoQ stream. | |
| Returns: | |
| str: The generated subscriber token. | |
| """ | |
| response = cast( | |
| MoqToken, | |
| self._request(create_moq_subscriber_token, stream_id=stream_name), | |
| def create_moq_publisher_token(self, stream_id: str) -> str: | |
| """Generates a MoQ publisher token for the given stream. | |
| Args: | |
| stream_id: The ID of the MoQ stream. | |
| Returns: | |
| str: The generated publisher token. | |
| """ | |
| response = cast( | |
| MoqToken, | |
| self._request(create_moq_publisher_token, stream_id=stream_id), | |
| ) | |
| return response.token | |
| def create_moq_subscriber_token(self, stream_id: str) -> str: | |
| """Generates a MoQ subscriber token for the given stream. | |
| Args: | |
| stream_id: The ID of the MoQ stream. | |
| Returns: | |
| str: The generated subscriber token. | |
| """ | |
| response = cast( | |
| MoqToken, | |
| self._request(create_moq_subscriber_token, stream_id=stream_id), |
| def create_moq_publisher_token(self, stream_name: str) -> str: | ||
| """Generates a MoQ publisher token for the given stream. | ||
|
|
||
| Args: | ||
| stream_name: The name of the MoQ stream. | ||
|
|
||
| Returns: | ||
| str: The generated publisher token. | ||
| """ |
There was a problem hiding this comment.
The PR description indicates a documentation update is required, but this PR doesn’t include any documentation changes. Either add the corresponding docs (recommended for these new public FishjamClient methods) or update the PR description/checklist if documentation is intentionally out of scope.
Description
Documentation impact
Types of changes
not work as expected)