Skip to content

[Cosmos] set startup to false on any refresh#40203

Merged
simorenoh merged 4 commits intomainfrom
startup-fix
Mar 27, 2025
Merged

[Cosmos] set startup to false on any refresh#40203
simorenoh merged 4 commits intomainfrom
startup-fix

Conversation

@simorenoh
Copy link
Copy Markdown
Member

@simorenoh simorenoh commented Mar 24, 2025

Customers not initializing their async client manually or with a context manager can be affected by too many health checks happening for their services, adding overhead latencies to their applications by having to handle all these additional requests. This change sets the startup attribute to False on the first refresh it sees as True, which prevents unnecessary health checks from occurring.

Copilot AI review requested due to automatic review settings March 24, 2025 18:29
@simorenoh simorenoh requested review from a team and annatisch as code owners March 24, 2025 18:29
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR prevents unnecessary health checks by setting the startup attribute to False during endpoint refreshes. Key changes include updating the changelog to document the behavior change and modifying the async global endpoint manager to set startup to False following an endpoint refresh.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
sdk/cosmos/azure-cosmos/CHANGELOG.md Added changelog entry documenting reduction of health checks on client startup
sdk/cosmos/azure-cosmos/azure/cosmos/aio/_global_endpoint_manager_async.py Updates the refresh process to set startup to False after a successful refresh
Comments suppressed due to low confidence (1)

sdk/cosmos/azure-cosmos/azure/cosmos/aio/_global_endpoint_manager_async.py:119

  • [nitpick] Ensure that setting self.startup to False at this point is intended for all cases; if the goal is to disable health checks after startup, the logic appears correct, but consider whether a more centralized or self-documenting approach might improve clarity.
self.startup = False

Comment thread sdk/cosmos/azure-cosmos/CHANGELOG.md Outdated
@azure-sdk
Copy link
Copy Markdown
Collaborator

API change check

API changes are not detected in this pull request.

Copy link
Copy Markdown
Member

@kushagraThapar kushagraThapar left a comment

Choose a reason for hiding this comment

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

thanks for the quick fix @simorenoh
We should think about what are the other code paths a customer could be missing if not calling the aenter() function and then fix other side effects if any.

Copy link
Copy Markdown
Member

@tvaron3 tvaron3 left a comment

Choose a reason for hiding this comment

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

LGTM

@simorenoh simorenoh merged commit 67f2f08 into main Mar 27, 2025
19 checks passed
@simorenoh simorenoh deleted the startup-fix branch March 27, 2025 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants