Adding Azure REST samples support #178

zikalino · 2018-09-14T07:18:52Z

Summary

Issue Type

New Feature

yungezz · 2018-09-14T07:22:11Z

package.json

@@ -31,7 +31,8 @@
    "onCommand:vscode-ansible.playbook-in-docker",
    "onCommand:vscode-ansible.playbook-in-localansible",
    "onCommand:vscode-ansible.ssh",
-    "onCommand:vscode-ansible.sync-folder-ssh"
+    "onCommand:vscode-ansible.sync-folder-ssh",
+    "onCommand:vscode-ansible.rest-api-samples"


suggest to rest-module-samples. which is related to ansible

yungezz · 2018-09-14T07:22:32Z

src/playbookManager.ts

@@ -5,7 +5,7 @@ import * as vscode from 'vscode';
 export class PlaybookManager {
    constructor() {
    }
-
+    


yungezz · 2018-09-14T07:23:03Z

src/restSamples.ts

+    }
+
+    public displayMenu() {
+        this.createRestApiCall();


why need a wrapper of displayMenu()?

yungezz · 2018-09-14T07:23:24Z

src/restSamples.ts

+    // - query list of api groups (compute, etc...)
+    // - user selects api group
+    public createRestApiCall() {
+        let __this = this;


why need private __this?

yungezz · 2018-09-14T07:23:56Z

src/restSamples.ts

+    public selectOperation(path: string) {
+
+        let operations = this.queryAll(path);
+        let __this = this;


no need of private this

yungezz · 2018-09-14T07:26:34Z

src/restSamples.ts

+
+                    examples.forEach(function(s, i, a) {
+                        items.push({
+                            'label': apiVersion + ' - Sample: ' + s.split('/').pop(),


as we discussed, remove the word 'Sample"here

yes, sorry, forgot....

yungezz · 2018-09-14T07:27:39Z

src/restSamples.ts

+                    pm.insertTask(playbook);
+                } else {
+                    let playbook = swaggerHandler.generateRestApiTasks(selection['path'], selection['method'],  selection['example']);
+                    pm.insertTask(playbook);


move pm.insertTask() out of if/else

yungezz · 2018-09-14T07:28:08Z

src/restSamples.ts

+        });
+    }
+
+    public getSpecificationLocation(cb) {


suggest use async func instead of callback to make coding style consistent

yungezz · 2018-09-14T07:28:31Z

src/restSamples.ts

+    }
+
+    public getSpecificationLocation(cb) {
+        let config = vscode.workspace.getConfiguration('ansible');


there's utility in utiltities.js: getCodeConfiguration

yungezz · 2018-09-14T07:28:51Z

src/restSamples.ts

+            cb(config.get('AzureRestAPISpecificationPath'));
+        } else {
+
+            this._outputChannel.append('\nConnecting to Cloud Shell.');


wrong comments

yungezz · 2018-09-14T07:29:30Z

src/restSamples.ts

+
+            this._outputChannel.append('\nConnecting to Cloud Shell.');
+            this._outputChannel.show();
+            const progress = utilities.delayedInterval(() => { this._outputChannel.append('.') }, 500);


progress should at end of below log message, otherwise you'll ...............Getting Azurexxxxx

src/restSamples.ts

yungezz · 2018-09-14T07:31:09Z

src/restSamples.ts

+                            for (var path in swagger.paths) {
+                                for (var method in swagger.paths[path]) {
+                                    // add only if there are examples
+                                    if (swagger.paths[path][method]['x-ms-examples'] != undefined) {


!swagger.paths[path][method]['x-ms-examples'] , to cover both undefined, null and empty check

yungezz · 2018-09-14T07:32:19Z

src/restSamples.ts

+        this.queryDirectory(specLocation + '/specification', false, "", cb);
+    }
+
+    private queryApiGroup(path, cb) {


the 2 function names confusing.. why not use queryDirectory and queryApiGroup?

src/swagger.ts

yungezz · 2018-09-14T07:35:06Z

src/swagger.ts

+        method = method.toLowerCase();
+        playbook += //"# https://docs.microsoft.com/en-us/rest/api/" + area + "/" + resource + "/" + internalMethod + '\r';
+                    "- name: Call REST API - " + this.swagger.paths[path][method]['operationId'] + "\r" +
+                    "\t" + ((method == 'get') ? "azure_rm_resource_facts" : "azure_rm_resource") + ":\r" +


pls replace \t with whitespace array constants

playbookManager is replacing tabs with spaces but it doesnt' really matter

tabs /whitespace is matter. :(

of course they matter, and they are replaced later :-)

this is a public api. you really want your user do tab replace by themselves?

I believe they are replaced automatically when inserting sample using code in playbookManager. I could recheck it again...

but anyway, I replaced them with spaces....

edit.insert(new vscode.Position(0, 0)….. in vscode works in such a way that all inserted tabs are replaced with spaces according to settings appropriate for current editor

src/swagger.ts

yungezz · 2018-09-17T05:05:06Z

src/playbookManager.ts

@@ -25,11 +25,11 @@ export class PlaybookManager {

    public insertTask(task: string) {
        let __this = this;
-
+        


pls do code formatting by 'shift+alt+f' to remove those whitesapces.

yungezz · 2018-09-17T05:05:58Z

src/playbookManager.ts

            if (vscode.window.activeTextEditor.document.getText() == "") {
-
+               


currently, the sample will be only inserted into utiltiled-1 doc, what if user want to insert into existing doc, focus position?

fi existing yaml document is open, new sample will be inserted at the end

src/restSamples.ts

yungezz · 2018-09-17T05:07:00Z

src/restSamples.ts

+        let operations = this.queryAll(path);
+        let items = [];
+
+        for (var key in operations) {


operations null/undefined check?

yungezz · 2018-09-17T05:18:47Z

src/restSamples.ts

+            let home: string = path.join(process.env[(process.platform == 'win32') ? 'USERPROFILE' : 'HOME'], 'azure-rest-api-specs');
+            clone("https://github.com/Azure/azure-rest-api-specs.git", home, null, (result) => {
+                progress.cancel();
+                if (result == undefined) {


pls use check if (!result) for undefined, null, empty check

src/restSamples.ts

yungezz · 2018-09-17T05:21:38Z

src/restSamples.ts

+    private queryDirectory(path: string, files: boolean, ext: string,  cb) {
+        // just use filesystem
+        try {
+            let dirEntries = fs.readdirSync(path);


check path exists before read?

that's why try is here

perhaps there should be a message and we could clear the configuration if not there?

so you'd like the caller handle when path not exists?

yungezz · 2018-09-17T05:23:17Z

src/swagger.ts

+        method = method.toLowerCase();
+        playbook += //"# https://docs.microsoft.com/en-us/rest/api/" + area + "/" + resource + "/" + internalMethod + '\r';
+                    "- name: Call REST API - " + this.swagger.paths[path][method]['operationId'] + "\r" +
+                    "\t" + ((method == 'get') ? "azure_rm_resource_facts" : "azure_rm_resource") + ":\r" +


tabs /whitespace is matter. :(

yungezz · 2018-09-17T06:55:10Z

src/restSamples.ts

+        let specLocation = await this.getSpecificationLocation();
+        __this.queryDirectory(specLocation + '/specification', false, "", function (groups) {
+            if (groups != null) {
+                vscode.window.showQuickPick(groups).then(selection => {


this is async function already, pls use await vscode.window.showQuickPick()

yungezz · 2018-09-17T06:56:11Z

src/restSamples.ts

+
+            this._outputChannel.append("Getting Azure REST API specifications.");
+            let clone = require('git-clone');
+            let home: string = path.join(process.env[(process.platform == 'win32') ? 'USERPROFILE' : 'HOME'], 'azure-rest-api-specs');


under .vscode to make it hide?

yungezz · 2018-09-17T06:56:41Z

src/restSamples.ts

+            this._outputChannel.append("Getting Azure REST API specifications.");
+            let clone = require('git-clone');
+            let home: string = path.join(process.env[(process.platform == 'win32') ? 'USERPROFILE' : 'HOME'], 'azure-rest-api-specs');
+            clone("https://github.com/Azure/azure-rest-api-specs.git", home, null, (result) => {


async function, pls use await instead of callback

this library only supports callback

then pls remove async on function declaration

yungezz · 2018-09-17T06:59:48Z

src/restSamples.ts

+                                        let operationId: string = swagger.paths[path][method].operationId;
+                                        let description: string = swagger.paths[path][method].description;
+
+                                        if (!description || description == '') description = "Description not available";


not show this to user, by empty

just show empty string?

yungezz · 2018-09-17T07:01:31Z

src/restSamples.ts

+    private queryDirectory(path: string, files: boolean, ext: string,  cb) {
+        // just use filesystem
+        try {
+            let dirEntries = fs.readdirSync(path);


so you'd like the caller handle when path not exists?

yungezz · 2018-09-17T07:02:14Z

src/swagger.ts

+
+        method = method.toLowerCase();
+        playbook += //"# https://docs.microsoft.com/en-us/rest/api/" + area + "/" + resource + "/" + internalMethod + '\r';
+                    "- name: Call REST API - " + this.swagger.paths[path][method]['operationId'] + "\r" +


sample for Rest API... ?

with sample link here?

yungezz · 2018-09-17T07:07:10Z

README.md

@@ -139,6 +140,24 @@ Enable syntax highlighting by adding `files.associations` in `settings.json`, to
 ```
 There's notification message at right side of status bar.

+## Samples for azure_rm_resource (preview)


yungezz · 2018-09-17T07:07:39Z

README.md

@@ -139,6 +140,24 @@ Enable syntax highlighting by adding `files.associations` in `settings.json`, to
 ```
 There's notification message at right side of status bar.

+## Samples for azure_rm_resource (preview)
+
+This option provides easy an access to the samples available from Azure REST API Specifications.


This option provides sample code snippets for [azure_rm_resource](link here) module, those samples are from Azure Rest API spec.

yungezz · 2018-09-17T07:08:20Z

README.md

+This option provides easy an access to the samples available from Azure REST API Specifications.
+
+To use, press **F1** and then select **Ansible: Samples for azure_rm_resource (PREVIEW)** option.
+You will be first asked for API group, then required operation. After selecting operation a submenu will be displayed where you can select particular API version and sample. Extension will create appropriate playbook based on **azure_rm_resource** or **azure_rm_resource_facts** module.


submenu -> pickup list

yungezz · 2018-09-17T07:11:50Z

README.md

+
+Note that all the samples generated by the extension are based on REST API samples from this repository:
+
+https://github.com/Azure/azure-rest-api-specs


If you meet error when using those samples, please file issues at xxxx/issues.

README.md

yungezz · 2018-09-17T07:48:14Z

src/restSamples.ts

+            this._outputChannel.append("Getting Azure REST API specifications.");
+            let clone = require('git-clone');
+            let home: string = path.join(process.env[(process.platform == 'win32') ? 'USERPROFILE' : 'HOME'], 'azure-rest-api-specs');
+            clone("https://github.com/Azure/azure-rest-api-specs.git", home, null, (result) => {


then pls remove async on function declaration

yungezz · 2018-09-17T07:48:32Z

src/restSamples.ts

+        }            
+    }
+
+    private async queryAll(path: string): Promise<{}> {


also remove async if there's no async operation in function

yungezz · 2018-09-17T07:48:44Z

src/restSamples.ts

+        return operations;
+    }
+
+    private async queryApiGroup(path): Promise<string[]> {


yungezz · 2018-09-17T07:49:13Z

src/restSamples.ts

+        } 
+    }
+
+    private async queryDirectory(path: string, files: boolean, ext: string): Promise<string[]> {


no async calls inside

zikalino · 2018-09-21T01:54:37Z

tested vsix on windows and mac.....

zikalino added 4 commits September 14, 2018 13:49

adding rest samples

ece25df

added git clone package

d36e077

cloning mostly

a40b12a

enable if

4b64227

yungezz reviewed Sep 14, 2018

View reviewed changes

src/restSamples.ts Outdated

}

public displayMenu() {

this.createRestApiCall();

Copy link

Contributor

yungezz Sep 14, 2018

Choose a reason for hiding this comment

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

why need a wrapper of displayMenu()?

yungezz reviewed Sep 14, 2018

View reviewed changes

src/restSamples.ts Outdated Show resolved Hide resolved

yungezz reviewed Sep 14, 2018

View reviewed changes

src/swagger.ts Show resolved Hide resolved

yungezz reviewed Sep 14, 2018

View reviewed changes

src/swagger.ts Outdated Show resolved Hide resolved

zikalino added 9 commits September 14, 2018 16:22

replaced playbook managed and a few other fixes

9f912e7

fixed config

852d9a2

minor fixes for downloading spec

64a2fda

exception handling in samples

f5a6ab2

small fixes

4667062

use utilities to configure

9e7db20

fixed configuratin and swagger

0d25849

one more swagger simplification

944502b

added default value

925cd65

zikalino added 2 commits September 17, 2018 12:21

fixes requested by catherine

d3cf3ea

updated main menu thingy

f361e7e

yungezz reviewed Sep 17, 2018

View reviewed changes

zikalino added 5 commits September 17, 2018 13:39

some checks

114bf33

one more check

e8c9f7c

fixed tabs

913f4fb

formatting

6588f19

async function

96aa6ba

yungezz reviewed Sep 17, 2018

View reviewed changes

added doc about rest

41386b5

yungezz reviewed Sep 17, 2018

View reviewed changes

zikalino added 2 commits September 17, 2018 15:16

doc fixes

26601ab

description not available

4606950

yungezz reviewed Sep 17, 2018

View reviewed changes

README.md Outdated Show resolved Hide resolved

zikalino added 6 commits September 17, 2018 15:34

changed to async

4ebfea9

removed this

a640261

Sample for Azure REST API

ae8dd93

separate notes

52a8a94

remove async stuff

9c7cabe

store rest api specs inside extension folder

0621e92

yungezz approved these changes Sep 17, 2018

View reviewed changes

zikalino added 2 commits September 18, 2018 06:06

fixed promise & directory for cloning samples

d487980

cloning rest api

80cf6c7

zikalino merged commit be9ef5b into master Sep 21, 2018

zikalino deleted the rest-samples branch September 21, 2018 01:54

		@@ -25,11 +25,11 @@ export class PlaybookManager {

		public insertTask(task: string) {
		let __this = this;

		if (vscode.window.activeTextEditor.document.getText() == "") {


		Note that all the samples generated by the extension are based on REST API samples from this repository:

		https://github.com/Azure/azure-rest-api-specs

Adding Azure REST samples support #178

Adding Azure REST samples support #178

Conversation

zikalino commented Sep 14, 2018

Summary

Issue Type

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

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

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

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

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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zikalino commented Sep 21, 2018