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.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 'Unclosed client session' error #830

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

beplay
Copy link
Contributor

@beplay beplay commented Apr 30, 2024

Motivation and Context

  1. Why is this change required?

To resolve Unclosed client session errors which occur when the Web App service is running as well as when the local start.sh script is used to run the application.

  1. What problem does it solve?

As per issues #751 and #593 improper handling of database and other asynchronous connections results in Unclosed client session errors.

  1. What scenario does it contribute to?

Using .aio libraries requires the proper handling and/or closing of asynchronous connections.

  1. If it fixes an open issue, please link to the issue here.

It resolves issues #751 and #593.

  1. Does this solve an issue or add a feature that all users of this sample app can benefit from? Contributions will only be accepted that apply across all users of this app.

It benefits all users who run the quart version of the app.

Description

As per the CosmosClient class in the azure-cosmos Python library, creating a database instance is a heavy operation:

... It's recommended to maintain a single instance of CosmosClient per lifetime of the application which enables efficient connection management and performance. CosmosClient initialization is a heavy operation, ...

To avoid multiple connection attempts, only a single global instance of the cosmos database connection is created in the init() function as part of the create_app() function. Only a single call to init_cosmosdb_client() is now needed instead of multiple calls within each function. This implementation leverages the current_app method of the quart library to maintain the database object's state during the lifetime of the application. When the database object is needed, it can be called using current_app, e.g., current_app.cosmos_conversation_client.create_message(...)

In addition, an Event is used to signal the readiness status of the database initialization to downstream functions to avoid the app calling methods of the CosmosConversationClient class before the client is completely initialized. Without this approach, a user would see an error on the frontend that disappears only once the initialization is complete. Functions can 'wait' on the database using the .wait() method of the Event class.

Another source of the Unclosed client session error stems from the use of the asynchronous import of DefaultAzureCredential from .aio. This connection is handled using a context created by the async with statement. Thanks to QuentinAd for the template.

Contribution Checklist

  • [ x ] I have built and tested the code locally and in a deployed app
  • [ x ] This is a change for all users of this app. No code or asset is specific to my use case or my organization.
  • [ x ] I didn't break any existing functionality 馃槃 馃

@github-actions github-actions bot added the stale label Jun 30, 2024
Copy link
Member

@abhahn abhahn left a comment

Choose a reason for hiding this comment

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

Hi @beplay , thanks for your contribution!

Looks great to me, the only issue is that the current branch is out of date with the base. Could you please pull in changes from main and then I can approve it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants