Fix Cosmos Samples by sajeetharan · Pull Request #18623 · Azure/azure-sdk-for-js

sajeetharan · 2021-11-10T08:03:26Z

Fix issue #17930

Update ../../dist-sem to @azure/cosmos
Migrate samples to V2
Enable strict mode on samples
SDK update

Note : We need to publish the SDK as the samples uses some of the exports which are not in live yet. SasTokenAuth.ts

azure-sdk · 2021-11-10T08:10:58Z

API changes have been detected in @azure/cosmos. You can review API changes here

simorenoh

Just had a couple questions, but it looks great! Good to have this fixed!

simorenoh · 2021-11-10T15:01:30Z

    "@azure/cosmos": "latest",
    "dotenv": "latest",
-    "@azure/identity": "^2.0.1",
+    "@azure/identity": "^1.1.0",


why are we lowering this version?

The sample tool automatically picks the version of @azure/identity that @azure/cosmos uses for its own samples. We can just manually undo this change or upgrade @azure/cosmos to identity v2.

@azure/cosmos Upgraded to identity v2

simorenoh · 2021-11-10T15:02:18Z

    "@azure/cosmos": "latest",
    "dotenv": "latest",
-    "@azure/identity": "^2.0.1",
+    "@azure/identity": "^1.1.0",


same question as other package.json

@azure/cosmos Upgraded to identity v2

simorenoh · 2021-11-10T15:08:32Z

-import { SasTokenPermissionKind } from "../../../dist-esm/common";
-import { createAuthorizationSasToken } from "../../../dist-esm/utils/SasToken";
-import { SasTokenProperties } from "../../../dist-esm/client/SasToken/SasTokenProperties";
+import { SasTokenProperties,SasTokenPermissionKind,createAuthorizationSasToken } from "@azure/cosmos";


since all the imports are from @azure/cosmos and so is the CosmosClient then why not have them in the same import statement?

I recommend running the "organize imports" source action in VSCode. It will combine and sort imports.

since these are tests , i have changed it to import from the src

witemple-msft

The samples themselves already look a lot better. There are several things I noticed looking through that we need to address first, and two high-level comments:

Files in the /samples folder at the root of the repo are deleted in this PR. You'll need to check them back out from the main branch to restore that directory.
There are a lot of uses of ?. which is good syntax but won't work in Node 12. I've shown an example of some patterns that you can use instead in the comments, but in a lot of cases you're working with an object that it would be an error of some kind if it were undefined, so if you notice you're doing x?.y a lot, and you really expect x to have a value, you can instead wrap the whole thing in an if:

if (x !== undefined) {
  // Body here
} else {
  // print an error message
}

witemple-msft · 2021-11-10T16:01:58Z

 }

+// @public (undocumented)
+export function getUserAgent(suffix?: string): string;


Function has no documentation comment. Are you certain this should be public?

witemple-msft · 2021-11-10T16:02:18Z

 }

+// @public (undocumented)
+export enum SasTokenPermissionKind {


All undocumented.

xirzec

I'm confused about the changes outside of the cosmos folder

xirzec

Thanks for fixing the deletion issue! 👍

issue addressed

check-enforcer · 2021-11-16T07:00:30Z

This pull request is protected by Check Enforcer.

What is Check Enforcer?

Check Enforcer helps ensure all pull requests are covered by at least one check-run (typically an Azure Pipeline). When all check-runs associated with this pull request pass then Check Enforcer itself will pass.

Why am I getting this message?

You are getting this message because Check Enforcer did not detect any check-runs being associated with this pull request within five minutes. This may indicate that your pull request is not covered by any pipelines and so Check Enforcer is correctly blocking the pull request being merged.

What should I do now?

If the check-enforcer check-run is not passing and all other check-runs associated with this PR are passing (excluding license-cla) then you could try telling Check Enforcer to evaluate your pull request again. You can do this by adding a comment to this pull request as follows:
/check-enforcer evaluate
Typically evaulation only takes a few seconds. If you know that your pull request is not covered by a pipeline and this is expected you can override Check Enforcer using the following command:
/check-enforcer override
Note that using the override command triggers alerts so that follow-up investigations can occur (PRs still need to be approved as normal).

What if I am onboarding a new service?

Often, new services do not have validation pipelines associated with them, in order to bootstrap pipelines for a new service, you can issue the following command as a pull request comment:
/azp run prepare-pipelines
This will run a pipeline that analyzes the source tree and creates the pipelines necessary to build and validate your pull request. Once the pipeline has been created you can trigger the pipeline using the following comment:
/azp run js - [service] - ci

witemple-msft · 2021-11-16T15:55:54Z

/azp run js - cosmosdb - ci

azure-pipelines · 2021-11-16T15:56:02Z

Azure Pipelines failed to run 1 pipeline(s).

witemple-msft · 2021-11-16T16:19:03Z

/azp run js - cosmosdb - ci

azure-pipelines · 2021-11-16T16:19:25Z

Azure Pipelines successfully started running 1 pipeline(s).

azure-sdk · 2021-11-16T16:24:53Z

API changes have been detected in @azure/cosmos. You can review API changes here

witemple-msft · 2021-11-16T17:34:17Z

@sajeetharan I made a few changes.

Moved assets/Data/Families.json to samples-dev/Data/Families.json and changed the way it is imported (the usage pattern that was there previously wouldn't work with the JavaScript layout, only TypeScript). I changed the sample configuration in package.json as well, and added json files to the includes in the tsconfig.json.
Rewrote all x?.y in samples-dev to some form of x && x.y.
Ran experimental generator. It passes fine with the above changes. The new generator uses a newer version of our code formatter, so it inserts commas in a few places where the old one doesn't.

witemple-msft · 2021-11-16T17:34:27Z

/azp run js - cosmosdb - tests

azure-pipelines · 2021-11-16T17:34:32Z

No pipelines are associated with this pull request.

witemple-msft

Thank you, @sajeetharan for your hard work on this.

deyaaeldeen · 2022-02-08T20:32:12Z

    "composite": false,
    "noEmit": true,
-    "strict": true
+    "strict": false


I wonder why this change was needed?

sajeetharan added 7 commits November 4, 2021 20:03

fix cosmos samples

2321d7d

add Families.json

203a20c

samples 'strict' mode fix

c3f87a7

add assets folder

81e622e

fixes on the strict mode issues

3509154

update assets path in samples-dev

657d739

add generated samples via dev tool

96d0942

sajeetharan requested review from bterlson, deyaaeldeen, jeremymeng, kushagraThapar, moderakh, ramya-rao-a, simorenoh, simplynaveen20, witemple-msft, xinlian12 and xirzec as code owners November 10, 2021 08:03

ghost added the Cosmos label Nov 10, 2021

simorenoh approved these changes Nov 10, 2021

View reviewed changes

witemple-msft requested changes Nov 10, 2021

View reviewed changes

xirzec previously requested changes Nov 10, 2021

View reviewed changes

Comment thread samples/Bundling/parcel/js/index.js

sajeetharan added 2 commits November 13, 2021 00:16

fix PR comments

66dc093

upgrade @azure/identity to identity v2

ec08333

xirzec reviewed Nov 15, 2021

View reviewed changes

fix lint issues

b267694

[cosmosdb] Experimental sample generation

cc53f67

witemple-msft approved these changes Nov 16, 2021

View reviewed changes

witemple-msft merged commit 308309a into Azure:main Nov 16, 2021

witemple-msft mentioned this pull request Nov 17, 2021

Azure Cosmos DB Samples Issue #17930

Closed

ramya-rao-a mentioned this pull request Dec 14, 2021

[cosmosdb] Migrate to Samples Workflow V2 #14468

Closed

1 task

deyaaeldeen reviewed Feb 8, 2022

View reviewed changes

deyaaeldeen mentioned this pull request Feb 8, 2022

[Cosmos] switch strict back on in src/tsconfig.json #20260

Merged

3 tasks

Conversation

sajeetharan commented Nov 10, 2021

Uh oh!

azure-sdk commented Nov 10, 2021

Uh oh!

simorenoh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

witemple-msft left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xirzec left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

xirzec left a comment

Choose a reason for hiding this comment

Uh oh!

check-enforcer Bot commented Nov 16, 2021

What is Check Enforcer?

Why am I getting this message?

What should I do now?

What if I am onboarding a new service?

Uh oh!

witemple-msft commented Nov 16, 2021

Uh oh!

azure-pipelines Bot commented Nov 16, 2021

Uh oh!

witemple-msft commented Nov 16, 2021

Uh oh!

azure-pipelines Bot commented Nov 16, 2021

Uh oh!

azure-sdk commented Nov 16, 2021

Uh oh!

witemple-msft commented Nov 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

witemple-msft commented Nov 16, 2021

Uh oh!

azure-pipelines Bot commented Nov 16, 2021

Uh oh!

witemple-msft left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

witemple-msft commented Nov 16, 2021 •

edited

Loading