Update Regions.cs - Adding new region for USSec #4381
Conversation
Adding new region for USSec
trande4884
requested review from
kirankumarkolli,
ealsur,
FabianMeiswinkel,
kirillg and
sboshra
as code owners
| /// <summary> | ||
| /// Name of the Azure USSec West Central region in the Azure Cosmos DB service. | ||
| /// </summary> | ||
| public const string USSecWestCentral = "USSec West Central"; |
There was a problem hiding this comment.
The current version of Cosmos.Direct package version 3.32.1 have the USSecWestCentral region marked as internal. See this code path for more details. This means that when the customer select this region, it will eventually fail during validation.
We will need to wait until a new version of Cosmos.Direct package is released which will mark this region as public. Can you use this TSG to create a new version of the direct package which contains the required changes please?
There was a problem hiding this comment.
@kundadebdatta how about adding a UT which iterates through all public and tries to use them for both ApplicationRegion and Preferred regions>
It will catch any gaps that might slip through right?
There was a problem hiding this comment.
Sounds great to me. That way, we could catch the invalid/ unavailable regions faster.
There was a problem hiding this comment.
@kirankumarkolli and @kundadebdatta Taylor and I are largely unfamiliar with this code, are you able to make the suggested changes to "catch gaps that might slip"? What is a "UT"? Some assistance on getting this done would be great!
There was a problem hiding this comment.
@trande4884 : "UT" basically stands for "Unit Test". While you work on the Direct package release, I can take this and update your PR to add the Unit Test to cover the scenario described.
There was a problem hiding this comment.
One more point - Can you please create a feature branch and submit a PR to address these changes. Forking is not supported for our pipelines and hence the PR will miss the necessary gates required for merging. Cc: @trande4884.
There was a problem hiding this comment.
With forking we need to explictly kick start the pipeline, I will do it. That's okey.
There was a problem hiding this comment.
#4386 is going to be the PR going forward. This PR can be abandoned / closed.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Adding new region for USSec
Pull Request Template
Description
Please include a summary of the change and which issue is fixed. Include samples if adding new API, and include relevant motivation and context. List any dependencies that are required for this change.
Type of change
Please delete options that are not relevant.
Closing issues
To automatically close an issue: closes #IssueNumber