[storage] Updates to samples for CI and local runs #6496

willmtemple · 2019-12-11T00:08:23Z

A lot of changes here. All addressing some different things. This is part of the work for #5679

added support for the JS samples to prep-samples (fixes part one of Samples run fails in CI #6362 )
We can run the samples again with npm run execute:samples. This command will prep, build, and run both the TS and JS samples (fixes part two of Samples run fails in CI #6362 )
The packages should be nicer to run locally and support dotenv
Should be fully ready for publication on docs.microsoft.com, though we are still not seeing the file-share samples there.
I made sure that the packages are really runnable, by zipping them up, unzipping them outside of the repo, and running npm install && npm run execute:all
There is a batch run command in the form of npm run execute:all, which replaces execute-samples.js and ensures that our CI process will mirror the user process
Fixed a couple of small bugs with the samples that only show up once they leave the repo
Added copyright headers to the sample files

… skip them

…f they exist

XiaoningLiu · 2019-12-11T09:54:04Z

Will this apply to queue and file-datalake?

richardpark-msft

Runnable samples!

richardpark-msft · 2019-12-11T00:18:49Z

sdk/storage/storage-file-share/samples/javascript/run.js

+  ["basic", basicMain],
+  ["customPipeline", customPipelineMain],
+  ["iterators-files-and-directories", iteratorsFilesAndDirectoriesMain],
+  // ["iterators-handles", iteratorsHandlesMain], // Must be populated with a pre-existing share/directory name to run


Can you just add in the name of the share to the sample.env and let the user fill them in?

Yes but it would still require that it already exists, so we would have to then have some external way to mask it out for CI. I think a better solution is to modify the sample so that it doesn't rely on any pre-existing resources.

I'm not understanding this bit:

I think a better solution is to modify the sample so that it doesn't rely on any pre-existing resources.

This is true but by the time the sample runs how/when the resource was created doesn't really matter, only that the environment variables point to the right thing. So it still seems like having those values specified in the sample.env makes sense to me.

SDK developers will create the identity once, the storage account once and just use that over and over (via .env)
CI will create the identity multiple times, the storage account multiple times and just use that over and over (by injecting environment variables directly).

Or am I missing how you want to do this?

All of the rest of the examples in the storage packages create and tear down their resources dynamically, and the only thing they need is an account. I think we should just have this sample do the same thing. What I'm going to do for now is just make the samples exit if they aren't configured, and so this one will run fine in the runner if the environment variables aren't set up. It'll just log a message and skip itself. I'll add the variables to sample.env for customers' sake, but our CI shouldn't have to provision a share and a directory with some open handles to run this sample since that's a lot for the CI harness to have to set up and I'm not even sure if it could do that.

sdk/storage/storage-file-share/samples/javascript/run.js

richardpark-msft · 2019-12-11T00:23:40Z

sdk/storage/storage-file-share/samples/typescript/sample.env

@@ -0,0 +1,9 @@
+# Used in most samples. Retrieve these values from a storage account in the Azure Portal.


For your identity tests you're welcome to copy the lines that I also copied from somewhere else:

azure-sdk-for-js/sdk/eventhub/event-hubs/samples/sample.env

Line 13 in 887e83a

# Used in the usingAadAuth.ts sample

That way they don't have to enter the credentials in the environment (or paste them into their source code).

I think we can standardize on something between: maybe a quick set of instructions and links to the docs for EnvironmentCredential in identity

common/scripts/prep-samples.js

willmtemple · 2019-12-11T18:28:12Z

@XiaoningLiu I'm working on Queue today, and it will apply to files-datalake eventually, but datalake isn't on my list of packages to take care of immediately.

jeremymeng · 2019-12-14T08:07:04Z

common/scripts/prep-samples.js

      writeFile: promisify(baseFS.writeFile)
    };
  })();

 /**
- * Breadth-first search for files ending in .ts, starting from `tsDir`
+ * Breadth-first search for files matching a given predicate
 *
 * @param {string} tsDir The root of the sample tree to search


nit: tsDir => dir

common/scripts/prep-samples.js

XiaoningLiu · 2019-12-16T02:38:30Z

sdk/storage/storage-queue/samples/javascript/package.json

+  "keywords": [
+    "Azure",
+    "Storage",
+    "File",


File [](start = 5, length = 4)

Minor. Should be "Queue", please check other package.json files

XiaoningLiu

HarshaNalluru · 2019-12-16T18:46:40Z

sdk/storage/storage-blob/samples/javascript/advanced.js

-const { AnonymousCredential, BlobServiceClient, newPipeline } = require("../.."); // Change to "@azure/storage-blob" in your package
+const { AnonymousCredential, BlobServiceClient, newPipeline } = require("@azure/storage-blob");
+
+const { runSample } = require("./sampleHelpers");


Not sure if this is addressed already.. this would confuse users?

Offline discussion with Will..

We are going to provide samples through docs.microsoft.com similar to below
https://docs.microsoft.com/en-ca/samples/browse/?products=azure-storage&languages=javascript%2Cnodejs

separate zipped folders for javascript and typescript samples - just like the Download Zip link here.

In the bullet-list of links in the readmes, docs.microsoft.com/samples link will be provided instead of the samples folder link

js samples are located at samples/javascript whereas the ts samples are at samples/typescript/src

To improve user-convenience of just copy/pasting one single sample, @willmtemple will be looking to remove ./sampleHelpers part of the samples

@jeremymeng

willmtemple · 2019-12-16T20:35:45Z

@HarshaNalluru most recent changes modify prep-samples.js so that in addition to changing the imports around, it also snips main().catch and anything occurring thereafter from the file. Makes it so sampleHelpers isn't required.

willmtemple · 2019-12-16T21:14:41Z

/azp run js - storage-queue - tests

azure-pipelines · 2019-12-16T21:14:54Z

Azure Pipelines successfully started running 1 pipeline(s).

HarshaNalluru · 2019-12-16T21:54:02Z

sdk/storage/storage-blob/package.json

    "extract-api": "tsc -p . && api-extractor run --local",
-    "execute:samples": "node execute-samples.js",
+    "execute:js-samples": "node ../../../common/scripts/run-samples.js samples/javascript/",
+    "execute:ts-samples": "node ../../../common/scripts/run-samples.js samples/typescript/dist/samples/typescript/src/",


[NIT]
samples/typescript/dist/samples/typescript/src/ ?
samples/typescript/dist/ is not enough?

No, because it is not using the rollup artifact. It is actually importing the package sources as modules, so the tree in samples/typescript/dist after compilation looks something like this:

dist/ - samples/ - typescript/ - src/ - basic.js - ... (compiled sample code) - src/ - StorageClient.js - ... (compiled client code)

An alternative would be to use ts-node rather than compile and run the samples, like the previous script was doing, but that has its own set of problems (would need to make the *-samples scripts their own package and add ts-node as a dependency, otherwise we would be requiring global ts-node -- more than I think I can get done before leaving for the holidays).

willmtemple · 2019-12-16T22:05:09Z

/azp run js - storage-blob - tests

azure-pipelines · 2019-12-16T22:05:20Z

Azure Pipelines successfully started running 1 pipeline(s).

willmtemple · 2019-12-16T22:32:37Z

/azp run js - storage-file-share - tests

azure-pipelines · 2019-12-16T22:32:48Z

Azure Pipelines successfully started running 1 pipeline(s).

willmtemple · 2019-12-16T23:02:16Z

/azp run js - storage-file-share - tests

azure-pipelines · 2019-12-16T23:02:27Z

Azure Pipelines successfully started running 1 pipeline(s).

willmtemple · 2019-12-16T23:25:09Z

This last yellow check looks like it's stuck. AzP says that all pipelines have passed, so I am merging this.

witemple-msft added 10 commits December 9, 2019 16:01

[storage-file-share] updated samples

2a5230b

[prep-samples] allow files that don't have a matching import and just…

dd98eb2

… skip them

[storage-file-share] typescript samples in shape

2fe35d9

[storage-file-share] made clean samples step clean up package locks i…

0d68724

…f they exist

[prep-samples] added support for javascript samples

72e49cb

[storage-file-share] Added dotenv and updated instructions

adb764c

[storage-file-share] Brought js samples in line with ts samples

160ab84

[storage-file-share] bugfixes to ts samples packages

8d1070d

[storage-file-share] NPM scripts to run samples

7951b5a

[storage-file-share] remove execute-samples.js and use new runners

03c7b22

willmtemple added Client This issue points to a problem in the data-plane of the library. Storage Storage Service (Queues, Blobs, Files) labels Dec 11, 2019

willmtemple requested a review from richardpark-msft December 11, 2019 00:08

willmtemple requested review from bterlson, HarshaNalluru, jeremymeng, jiacfan, KarishmaGhiya, mikeharder, vinjiang and XiaoningLiu as code owners December 11, 2019 00:08

willmtemple self-assigned this Dec 11, 2019

witemple-msft added 3 commits December 10, 2019 16:17

[storage-file-share] added missing sample.env to js samples

6adcb5d

[storage-file-share] Fixed sample package names

9a793e6

[storage-blob] update samples to be like storage-file-share

ef9a3d4

ramya-rao-a requested a review from xirzec December 11, 2019 04:41

richardpark-msft reviewed Dec 11, 2019

View reviewed changes

[samples] Moved run-samples to common/scripts

6ac61c7

jeremymeng reviewed Dec 14, 2019

View reviewed changes

common/scripts/prep-samples.js Show resolved Hide resolved

XiaoningLiu reviewed Dec 16, 2019

View reviewed changes

XiaoningLiu approved these changes Dec 16, 2019

View reviewed changes

witemple-msft added 2 commits December 16, 2019 10:20

[storage] Fixed wrong keywords in three sample packages.

787e631

[prep-samples] tsDir -> dir

1e25ece

HarshaNalluru reviewed Dec 16, 2019

View reviewed changes

willmtemple mentioned this pull request Dec 16, 2019

Update samples readme for each package #5679

Closed

3 tasks

witemple-msft added 3 commits December 16, 2019 12:06

[storage-queue] Removed need for sampleHelpers

d6653e5

[storage-file-share] Remove sampleHelpers

e10e99b

[storage-blob] removed sampleHelpers

3145d3c

willmtemple requested review from HarshaNalluru and jeremymeng December 16, 2019 20:36

[run-samples] Removed outdated sampleHelpers.js from ignore list

3be4483

HarshaNalluru reviewed Dec 16, 2019

View reviewed changes

HarshaNalluru approved these changes Dec 16, 2019

View reviewed changes

willmtemple merged commit 47b0535 into Azure:master Dec 16, 2019

HarshaNalluru mentioned this pull request Dec 16, 2019

Samples run fails in CI #6362

Closed

2 tasks

willmtemple mentioned this pull request Dec 18, 2019

[keyvault] samples updates for doc publishing #6626

Merged

ljian3377 mentioned this pull request Jan 20, 2020

[storage][sample]fix sample debug in launch.json #7030

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[storage] Updates to samples for CI and local runs #6496

[storage] Updates to samples for CI and local runs #6496

willmtemple commented Dec 11, 2019

XiaoningLiu commented Dec 11, 2019

richardpark-msft left a comment

richardpark-msft Dec 11, 2019

willmtemple Dec 11, 2019

richardpark-msft Dec 11, 2019

willmtemple Dec 11, 2019

richardpark-msft Dec 11, 2019

willmtemple Dec 11, 2019

willmtemple commented Dec 11, 2019

jeremymeng Dec 14, 2019

XiaoningLiu Dec 16, 2019

XiaoningLiu left a comment

HarshaNalluru Dec 16, 2019

HarshaNalluru Dec 16, 2019

willmtemple commented Dec 16, 2019

willmtemple commented Dec 16, 2019

azure-pipelines bot commented Dec 16, 2019

HarshaNalluru Dec 16, 2019

willmtemple Dec 16, 2019 •

edited

Loading

willmtemple commented Dec 16, 2019

azure-pipelines bot commented Dec 16, 2019

willmtemple commented Dec 16, 2019

azure-pipelines bot commented Dec 16, 2019

willmtemple commented Dec 16, 2019

azure-pipelines bot commented Dec 16, 2019

willmtemple commented Dec 16, 2019

		@@ -0,0 +1,9 @@
		# Used in most samples. Retrieve these values from a storage account in the Azure Portal.

[storage] Updates to samples for CI and local runs #6496

[storage] Updates to samples for CI and local runs #6496

Conversation

willmtemple commented Dec 11, 2019

XiaoningLiu commented Dec 11, 2019

richardpark-msft left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

willmtemple commented Dec 11, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

XiaoningLiu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

willmtemple commented Dec 16, 2019

willmtemple commented Dec 16, 2019

azure-pipelines bot commented Dec 16, 2019

Choose a reason for hiding this comment

willmtemple Dec 16, 2019 • edited Loading

Choose a reason for hiding this comment

willmtemple commented Dec 16, 2019

azure-pipelines bot commented Dec 16, 2019

willmtemple commented Dec 16, 2019

azure-pipelines bot commented Dec 16, 2019

willmtemple commented Dec 16, 2019

azure-pipelines bot commented Dec 16, 2019

willmtemple commented Dec 16, 2019

willmtemple Dec 16, 2019 •

edited

Loading