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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using connection mode from Cosmos Configuration #4807

Merged
merged 1 commit into from
Jan 29, 2025

Conversation

rajithaalurims
Copy link
Contributor

Description

Default connection mode in Cosmos Configuration is "Direct". This PR has changes to use connection mode based on the value in configuration file.

Related issues

Addresses [issue # 140094].

Testing

Confirmed that the connection mode is set based on the configuration file.

FHIR Team Checklist

  • Update the title of the PR to be succinct and less than 65 characters
  • Add a milestone to the PR for the sprint that it is merged (i.e. add S47)
  • Tag the PR with the type of update: Bug, Build, Dependencies, Enhancement, New-Feature or Documentation
  • Tag the PR with Open source, Azure API for FHIR (CosmosDB or common code) or Azure Healthcare APIs (SQL or common code) to specify where this change is intended to be released.
  • Tag the PR with Schema Version backward compatible or Schema Version backward incompatible or Schema Version unchanged if this adds or updates Sql script which is/is not backward compatible with the code.
  • CI is green before merge Build Status
  • Review squash-merge requirements

Semver Change (docs)

Patch|Skip|Feature|Breaking (reason)

@rajithaalurims rajithaalurims added the Azure API for FHIR Label denotes that the issue or PR is relevant to the Azure API for FHIR label Jan 29, 2025
@rajithaalurims rajithaalurims added this to the 2Wk07 milestone Jan 29, 2025
@rajithaalurims rajithaalurims requested a review from a team as a code owner January 29, 2025 15:44
@rbans96
Copy link
Contributor

rbans96 commented Jan 29, 2025

Looks good - we should monitor latency with this change as it adds an extra hop since requests go through the gateway.

@@ -131,8 +131,16 @@ private CosmosClient CreateCosmosClientInternal(CosmosDataStoreConfiguration con
new CosmosClientBuilder(host, _cosmosAccessTokenProviderFactory.Invoke().TokenCredential) :
new CosmosClientBuilder(host, key);

if (configuration.ConnectionMode == ConnectionMode.Gateway)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add a logging statement to ensure that connectionMode is correctly populated from our config? If there are other means to confirm that, feel free to ignore

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In case of exceptions, we do see client configuration which displays connection mode.
Example: "ConnectionMode":"Direct"
To save time of PR build, I am going to leave it as it is.

@rajithaalurims rajithaalurims changed the title ConnectionMode is fetched from configuration instead of using Direct Mode Using connection mode from Cosmos Configuration Jan 29, 2025
@rajithaalurims rajithaalurims added the Bug Bug bug bug. label Jan 29, 2025
@rajithaalurims rajithaalurims merged commit 1f585f7 into main Jan 29, 2025
48 of 52 checks passed
@rajithaalurims rajithaalurims deleted the personal/rajitha/cosmos-connection-mode branch January 29, 2025 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure API for FHIR Label denotes that the issue or PR is relevant to the Azure API for FHIR Bug Bug bug bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants