Add integration tests for connecthealth#62
Conversation
| bucket_name: The name of the S3 bucket to create. | ||
| """ | ||
| s3_client.create_bucket(Bucket=bucket_name) | ||
| s3_client.put_bucket_tagging( |
There was a problem hiding this comment.
[nit]
Two separate http calls means we could create the bucket but fail to tag it. We can include tags on the initial request via CreateBucketConfiguration. See the docs for the syntax.
| subscription_id: The Subscription ID to poll. | ||
| """ | ||
| deadline = asyncio.get_event_loop().time() + _SUBSCRIPTION_POLL_TIMEOUT_SECONDS | ||
| while asyncio.get_event_loop().time() < deadline: |
There was a problem hiding this comment.
[nit/question]
Did we consider using asyncio.timeout here? Something like this:
async with asyncio.timeout(_SUBSCRIPTION_POLL_TIMEOUT_SECONDS):
while True:
response = await client.get_subscription(
input=GetSubscriptionInput(
domain_id=domain_id, subscription_id=subscription_id
)
)
if (
response.subscription is not None
and response.subscription.status == SubscriptionStatus.INACTIVE
):
return
await asyncio.sleep(_SUBSCRIPTION_POLL_INTERVAL_SECONDS)
IMO that's a little more readable and effectively the same, but it's personal preference. Though the above will cancel a request while it's in flight, and the existing code always finishes the given iteration.
There was a problem hiding this comment.
Thanks for the suggestion! I considered asyncio.timeout, but I'd lean towards keeping the current approach. As you noted, it always finishes the given iteration rather than cancelling it mid-request, which feels a bit safer.
That said, on a related comment in another PR we did switch from asyncio.get_event_loop() to asyncio.get_running_loop(). So I also applied the switch for this PR in 62eb957. In this case, it also keeps things consistent with the other existing tests.
| input=GetMedicalScribeListeningSessionInput( | ||
| session_id=session_id, domain_id=domain_id, subscription_id=subscription_id | ||
| ), | ||
| plugins=[streaming_plugin], |
There was a problem hiding this comment.
[minor nit]
This confused me a bit since this isn't a streaming operation, but it still uses the streaming endpoint. Maybe naming it streaming_endpoint_plugin would be easier to read, but this one is really not that big of a deal.
There was a problem hiding this comment.
Thanks, agreed there's some confusion here. However, naming it streaming_endpoint_plugin directly would shadow the imported function of the same name and cause an UnboundLocalError on the following line:
streaming_endpoint_plugin = streaming_endpoint_plugin(REGION)
so I renamed it to endpoint_plugin instead to reduce the confusion. Done in 62eb957.
d69b5ba to
363ba4f
Compare
Description of Changes
This PR adds integration tests for the Connect Health service client (
aws-sdk-connecthealth), covering the following:create_domain,list_domains,get_domain,delete_domain,create_subscription,list_subscriptions,get_subscription,deactivate_subscription,get_medical_scribe_listening_session.start_medical_scribe_listening_session.The test structure and patterns follow the same conventions established in the Lex Runtime V2 integration tests (PR #51), such as session-scoped conftest fixtures for resource setup/teardown.
Testing
The integration tests create real AWS resources (Domain, Subscription, S3 bucket) under the calling AWS identity, run a live streaming session against the Connect Health service, then delete those resources. The calling identity therefore needs Admin (or equivalent) permissions on ConnectHealth and S3.
Assume an Admin role and export the resulting credentials so the new SDK (which uses
EnvironmentCredentialsResolver) can pick them up:From
clients/aws-sdk-connecthealth/, install the client and test dependencies:Run the tests
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.