VAPI-2714 Update the python SDK to include BRTC endpoint APIs#280
VAPI-2714 Update the python SDK to include BRTC endpoint APIs#280
Conversation
- Introduced `SipCredentials` model for managing SIP connection credentials. - Added documentation for `CreateEndpointRequestBase`, `CreateEndpointResponse`, `CreateEndpointResponseObject`, `CreateWebRtcConnectionRequest`, `Device`, `DeviceStatusEnum`, `Endpoint`, `EndpointDirectionEnum`, `EndpointEvent`, `EndpointEventTypeEnum`, `EndpointResponse`, `EndpointStatusEnum`, `EndpointTypeEnum`, `Endpoints`, `EndpointsApi`, `ErrorResponse`, `ListEndpointsResponse`, `Page`, `SipConnectionMetadata`, and `SipCredentials`. - Implemented example usage in the documentation for each model.
- Implement integration tests for creating, retrieving, listing, and deleting WebRTC endpoints in `test/smoke/test_endpoints_api.py`. - Add unit tests for models related to endpoint requests and responses, including `CreateEndpointRequestBase`, `CreateEndpointResponse`, `CreateEndpointResponseObject`, `Device`, and their respective enums in the `test/unit/models` directory. - Ensure tests cover various scenarios including unauthorized and forbidden access attempts. - Validate the functionality of endpoint events and status enums with dedicated unit tests.
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
…on fields in ListEndpointsResponse tests
… is not ready yet
…emove forbidden credentials test
…WebRtcConnectionRequest, and Endpoint classes
…reateWebRtcConnectionRequest, and Endpoint classes
smoghe-bw
changed the title
…-sdk into brtc-python-sdk
test/smoke/test_endpoints_api.py
Outdated
|
|
||
| def test_create_endpoint(self): | ||
| """Test creating a new WebRTC endpoint with all parameters""" | ||
| time.sleep(self.TEST_SLEEP) |
There was a problem hiding this comment.
why do we need to sleep before every test? this seems like its just making the smoke tests take longer for no reason
test/smoke/test_endpoints_api.py
Outdated
| time.sleep(self.TEST_SLEEP) | ||
|
|
||
| # First create an endpoint | ||
| create_request = CreateWebRtcConnectionRequest( |
There was a problem hiding this comment.
do we need to create a new one or can we just use the endpoint id from the create test?
test/smoke/test_endpoints_api.py
Outdated
| # Store endpoint ID for cleanup | ||
| self.endpoint_id_array.append(response.data.data.endpoint_id) | ||
|
|
||
| def test_create_endpoint_minimal(self): |
There was a problem hiding this comment.
typically we try to smoke test using as much of the SDK as possible, since the tests in here are meant to test SDK functionality, this test can probably be removed.
test/smoke/test_endpoints_api.py
Outdated
| endpoint_id = create_response.data.endpoint_id | ||
| self.endpoint_id_array.append(endpoint_id) | ||
|
|
||
| time.sleep(self.TEST_SLEEP) |
There was a problem hiding this comment.
is there a delay on these? all of these sleeps seem like they're mimicking the calls api tests, which need to wait due to latency issues, is that the case here as well?
|
|
||
| assert_that(response.status_code, equal_to(201)) | ||
| assert_that(response.data, instance_of(CreateEndpointResponse)) | ||
| assert_that(response.data.data, has_properties( |
There was a problem hiding this comment.
can we add more assertions here? specifically testing all of links, data, and errors, as well as the properties on those. There are also some other properties on CreateEndpointResponseData we could be asserting here
| # | ||
| # assert_that(response.status_code, equal_to(204)) | ||
|
|
||
| def test_create_endpoint_minimal(self) -> None: |
| assert_that(response.status_code, equal_to(201)) | ||
| assert_that(response.data, instance_of(CreateEndpointResponse)) | ||
|
|
||
| def test_create_endpoint_sip(self) -> None: |
There was a problem hiding this comment.
also unnecessary, this may be something we want to test in the smoke tests to make sure the API behaves properly, but IMO we shouldnt be testing API functionality in the SDKs, so we can just leave this out
| assert instance.event_fallback_url == 'https://example.com/fallback' | ||
| assert instance.tag == 'test-request' | ||
|
|
||
| def testCreateEndpointRequestBaseRequiredOnly(self): |
There was a problem hiding this comment.
we really only care that all of the fields can be serialized, testing the required only props does nothing the full test doesn't already do, this can be removed
| assert instance.status == DeviceStatusEnum.CONNECTED | ||
| assert isinstance(instance.creation_timestamp, datetime) | ||
|
|
||
| def testDeviceRequiredOnly(self): |
There was a problem hiding this comment.
this can be removed, see my comment about it in the create_endpoint_base test
it can also be removed from any following model tests
There was a problem hiding this comment.
only some of the new models added by this PR are unit tested here, we need to have tests for all of the new models. If you remove test/* temporarily from the openpi-generator-ignore it'll generate test files for all of the models, and you can pick out the new ones as a base for the ones still missing here
3 participants
No description provided.