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.

Sign up for GitHub

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

Make Go2RtcRestClient.validate_server_version a classmethod #17

Closed
wants to merge 2 commits into from
Closed

Make Go2RtcRestClient.validate_server_version a classmethod #17

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
776173f
Make Go2RtcRestClient.validate_server_version a classmethod
emontnemery Oct 29, 2024
5540ea2
Lint code
emontnemery Oct 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions go2rtc_client/rest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,15 @@ def __init__(self, websession: ClientSession, server_url: str) -> None:
self.streams: Final = _StreamClient(self._client)
self.webrtc: Final = _WebRTCClient(self._client)

@classmethod
@handle_error
async def validate_server_version(self) -> None:
async def validate_server_version(
cls, websession: ClientSession, server_url: str
) -> None:
Copy link
Contributor

@MartinHjelmare MartinHjelmare Oct 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really more convenient? Won't this just mean we have to pass the session and url twice to two calls instead of passing them once to the first of two calls, when validating the server version and creating a client? We also just save two instance creation calls out of four internally compared to creating the client from the beginning.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had the same discussion with Robert. From the user's point of view, I think it makes sense to make it a class method; there's no need for a client if the server has the wrong version.
From the current implementation point of view it's as you say currently not useful since the implementation creates the client objects.
At the same time, the client objects are very lightweight and cost very little to create.

"""Validate the server version is compatible."""
application_info = await self.application.get_info()
client = _BaseClient(websession, server_url)
application = _ApplicationClient(client)
application_info = await application.get_info()
try:
version_supported = (
_MIN_VERSION_SUPPORTED
Expand Down
10 changes: 5 additions & 5 deletions tests/test_rest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@
import json
from typing import TYPE_CHECKING, Any

import aiohttp
from aiohttp.hdrs import METH_PUT
from awesomeversion import AwesomeVersion
import pytest

from go2rtc_client import Go2RtcRestClient
from go2rtc_client.exceptions import Go2RtcVersionError
from go2rtc_client.models import WebRTCSdpOffer
from go2rtc_client.rest import _ApplicationClient, _StreamClient, _WebRTCClient
Expand All @@ -21,8 +23,6 @@
from aioresponses import aioresponses
from syrupy import SnapshotAssertion

from go2rtc_client import Go2RtcRestClient


async def test_application_info(
responses: aioresponses,
Expand Down Expand Up @@ -112,7 +112,6 @@ async def test_streams_add(
)
async def test_version_supported(
responses: aioresponses,
rest_client: Go2RtcRestClient,
server_version: str,
expected_result: AbstractContextManager[Any],
) -> None:
Expand All @@ -124,8 +123,9 @@ async def test_version_supported(
status=200,
payload=payload,
)
with expected_result:
await rest_client.validate_server_version()
async with aiohttp.ClientSession() as session:
with expected_result:
await Go2RtcRestClient.validate_server_version(session, URL)


async def test_webrtc_offer(
Expand Down