serviceconnector: Add Service Connector 🔗 #1476

motm32 · 2023-05-25T21:31:50Z

Part of #1419.

This will currently be used with the Container Apps and App Service extensions. The current implementation has picks for only storage connections.

Still to do: (these will go in seperate PR's)

Add UI
Add database connections

serviceconnector/package.json

serviceconnector/src/createServiceConnector/ServiceTypeStep.ts

serviceconnector/src/createServiceConnector/ICreateLinkerContext.ts

serviceconnector/constants.ts

serviceconnector/src/createServiceConnector/ClientTypeStep.ts

serviceconnector/src/createServiceConnector/ICreateLinkerContext.ts

serviceconnector/src/createServiceConnector/AuthenticationTypeStep.ts

serviceconnector/src/createServiceConnector/ICreateLinkerContext.ts

serviceconnector/package.json

serviceconnector/src/createServiceConnector/AuthenticationTypeStep.ts

serviceconnector/src/createServiceConnector/LinkerNameStep.ts

serviceconnector/src/deleteServiceConnector/ServiceConnectorsListStep.ts

serviceconnector/src/validateServiceConnector/ValidateLinkerStep.ts

serviceconnector/src/deleteServiceConnector/LinkerListStep.ts

serviceconnector/src/deleteServiceConnector/deleteLinker.ts

serviceconnector/src/validateServiceConnector/ValidateLinkerStep.ts

serviceconnector/src/createServiceConnector/createLinker.ts

serviceconnector/src/createServiceConnector/TargetServiceTypeStep.ts

serviceconnector/src/createServiceConnector/AuthenticationTypeStep.ts

serviceconnector/src/createServiceConnector/ICreateLinkerContext.ts

MicroFish91 · 2023-06-08T17:38:01Z

serviceconnector/src/createServiceConnector/ICreateLinkerContext.ts

+import { ISubscriptionActionContext } from "@microsoft/vscode-azext-utils";
+import { TargetServiceType } from "../../constants";
+
+


Nit: extra new line

MicroFish91 · 2023-06-08T17:41:44Z

serviceconnector/src/createServiceConnector/TargetServiceTypeStep.ts

+            { label: targetServiceLabels[0], data: { name: "storageBlob", type: TargetServiceTypeName.storage, id: '/blobServices/default' } },
+            { label: targetServiceLabels[1], data: { name: "storageQueue", type: TargetServiceTypeName.storage, id: '/queueServices/default' } },
+            { label: targetServiceLabels[2], data: { name: "storageTable", type: TargetServiceTypeName.storage, id: '/tableServices/default' } },
+            { label: targetServiceLabels[3], data: { name: "storageFile", type: TargetServiceTypeName.storage, id: '/fileServices/default' } },


Nit, but the other enums like StorageAccountKind use properties that start with a capital letter, so I think it would look better to do the same, i.e. TargetServiceTypeName.Storage

MicroFish91 · 2023-06-08T17:53:25Z

Also... would it make sense to add activity log support for createLinker/deleteLinker/validateLinker?

motm32 · 2023-06-08T18:02:53Z

Also... would it make sense to add activity log support for createLinker/deleteLinker/validateLinker?

Yup already planning to add it :). I originally was planning to just do it in the client extensions but ran into some issues with that.

nturinski · 2023-06-08T19:51:10Z

serviceconnector/src/createServiceConnector/ClientTypeStep.ts

+export class ClientTypeStep extends AzureWizardPromptStep<ICreateLinkerContext>{
+    public async prompt(context: ICreateLinkerContext): Promise<void> {
+        const placeHolder = vscode.l10n.t('Select Language/Framework');
+        const picks: IAzureQuickPickItem<KnownClientType>[] = [


Is this ordering the same as the portal? It feels a little random/arbitrary.

I'd think we should either do it alphabetically or by what we expect to be the most common choices. That being said, I think it'd be cool if client extensions have a way to recommend/organize these picks.

For example, for a Function App, it would be very easy for us to see what runtime their app is running and have it be at the top.

This is the order the portal does except they exclude PHP and Ruby. I do think that the portal has some smart detection where they will pre fill in a client type for you based on the runtime of your app which would be helpful to add.

I'd opt for just putting it in alphabetical order so it makes some sense.

Maybe have an optional argument of KnownClientType[] so those types can get put to the top as part of the "recommended" group or something?

serviceconnector/src/createServiceConnector/ICreateLinkerContext.ts

serviceconnector/src/createServiceConnector/TargetServiceTypeStep.ts

serviceconnector/src/deleteServiceConnector/deleteLinker.ts

nturinski · 2023-06-12T16:49:13Z

serviceconnector/src/createServiceConnector/TargetServiceTypeStep.ts

+import { TargetServiceType, TargetServiceTypeName } from "../../constants";
+import { ICreateLinkerContext } from "./ICreateLinkerContext";
+
+export class TargetServiceTypeStep extends AzureWizardPromptStep<ICreateLinkerContext>{


Also just following other conventions, but this is generally called a ListStep.

nturinski · 2023-06-12T16:52:12Z

serviceconnector/src/createServiceConnector/CreateLinkerStep.ts

+import { createServiceConnectorClient } from "../serviceConnectorClient";
+import { ICreateLinkerContext } from "./ICreateLinkerContext";
+
+export class CreateLinkerStep extends AzureWizardExecuteStep<ICreateLinkerContext>{


nit: Generally, we call things <noun>CreateStep. Just makes it easier to organize/read (otherwise they would all start with Create.

nturinski · 2023-06-12T16:54:11Z

serviceconnector/src/createServiceConnector/ClientTypeStep.ts

+export class ClientTypeStep extends AzureWizardPromptStep<ICreateLinkerContext>{
+    public async prompt(context: ICreateLinkerContext): Promise<void> {
+        const placeHolder = vscode.l10n.t('Select Language/Framework');
+        const picks: IAzureQuickPickItem<KnownClientType>[] = [


I'd opt for just putting it in alphabetical order so it makes some sense.

Maybe have an optional argument of KnownClientType[] so those types can get put to the top as part of the "recommended" group or something?

nturinski · 2023-06-12T16:58:02Z

serviceconnector/src/createServiceConnector/createLinker.ts

+        new AuthenticationTypeStep(),
+    ];
+
+    promptSteps.unshift(...nonNullValue(preSteps));


You shouldn't assume that this is a nonNullValue. It'll cause problems in the event no prompt steps are passed through.

You should could probably just make preSteps default to an empty array:

export async function createLinker(context: IActionContext & ExecuteActivityContext, item: LinkerItem | AzExtTreeItem, preSteps: AzureWizardPromptStep<ICreateLinkerContext>[] = []): Promise<void>

serviceconnector/package.json

bwateratmsft · 2023-06-16T16:43:40Z

This will need to be added here in order to pass the PR validation. Also, can you merge main into your branch and push?

…ganmott/serviceConnector

alexweininger · 2023-06-20T20:59:57Z

serviceconnector/package.json

+        "mocha": "^9.1.3",
+        "mocha-junit-reporter": "^2.0.2",
+        "mocha-multi-reporters": "^1.1.7",
+        "typescript": "^4.9.4"


Might as well update to the latest version of TS

alexweininger · 2023-06-20T21:04:42Z

serviceconnector/src/createServiceConnector/ICreateLinkerContext.ts

+export interface ICreateLinkerContext extends ISubscriptionActionContext, IStorageAccountWizardContext {
+    //Source resource
+    sourceResourceUri?: string;
+    scope?: string; // This is only assigned when the source resource is a container app to indicate which container is being connected


If you use a multi-line comment (/** */ kind) then VS Code will pick it up as documentation. I think you should change this to a multi-line comment to leverage this.

alexweininger · 2023-06-20T21:05:52Z

serviceconnector/src/createServiceConnector/ICreateLinkerContext.ts

+    runtime?: KnownClientType[];
+    clientType?: KnownClientType;


Are these properties specific to app service? If so, add a comment to add some context

runtime is so I will add a comment about that being specific to app service but clientType will always be set. In the case of container apps it is set to None but is still being set.

alexweininger · 2023-06-20T21:10:05Z

serviceconnector/src/createServiceConnector/createLinker.ts

+}
+
+export async function createLinker(context: IActionContext & ExecuteActivityContext, item: LinkerItem | AzExtTreeItem, preSteps: AzureWizardPromptStep<ICreateLinkerContext>[] = [], runtime?: KnownClientType[]): Promise<void> {
+    const subscription = item instanceof AzExtTreeItem ? item.subscription : createSubscriptionContext(item.subscription); // For v1.5 compatibility


Should be using isAzExtTreeItem() here instead of instanceof. Classes are unique per instance of the utils package.

alexweininger · 2023-06-20T21:12:30Z

serviceconnector/src/createServiceConnector/createLinker.ts

+    id: string,
+}
+
+export async function createLinker(context: IActionContext & ExecuteActivityContext, item: LinkerItem | AzExtTreeItem, preSteps: AzureWizardPromptStep<ICreateLinkerContext>[] = [], runtime?: KnownClientType[]): Promise<void> {


preSteps isn't optional, but it's not clear to me (just from looking at this method) what the preSteps need to accomplish in order to work properly. Are there specific properties on ICreateLinkerContext the preSteps should define?

If so, a comment would go a long way here.

preSteps is referring to steps that are specific to the client extension that should to be set before any of the general service connector steps. For example in container apps we need to set the container. I had originally had this as an optional parameter but ran into issues with that since I was using nonNullValue which caused an error when testing with app service since there are no pre steps for that service. For more context here is the comment Nathan left:

You shouldn't assume that this is a nonNullValue. It'll cause problems in the event no prompt steps are passed through.

You should could probably just make preSteps default to an empty array:

export async function createLinker(context: IActionContext & ExecuteActivityContext, item: LinkerItem | >AzExtTreeItem, preSteps: AzureWizardPromptStep<ICreateLinkerContext>[] = []): Promise<void>

Oh I'm sorry, I totally missed that it was defaulting to an empty array. That makes more sense.

alexweininger · 2023-06-20T21:13:40Z

serviceconnector/src/deleteServiceConnector/LinkerListStep.ts

+
+export class LinkerListStep extends AzureWizardPromptStep<IPickLinkerContext>{
+    public async prompt(context: IPickLinkerContext): Promise<void> {
+        const placeHolder = vscode.l10n.t("Select a service connector");


nit: create the value inline where it's used below.

alexweininger · 2023-06-20T21:14:02Z

serviceconnector/src/deleteServiceConnector/deleteLinker.ts

+import { LinkerListStep } from "./LinkerListStep";
+
+export async function deleteLinker(context: IActionContext & ExecuteActivityContext, item: LinkerItem | AzExtTreeItem, preSteps: AzureWizardPromptStep<IPickLinkerContext>[] = []): Promise<void> {
+    const subscription = item instanceof AzExtTreeItem ? item.subscription : createSubscriptionContext(item.subscription); // For v1.5 compatibility


Use isAzExtTreeItem() instead of instanceof.

alexweininger · 2023-06-20T21:18:03Z

serviceconnector/src/validateServiceConnector/ValidateLinkerStep.ts

+        const response = await client.linker.beginValidateAndWait(nonNullValue(context.sourceResourceUri), nonNullValue(context.linkerName));
+        for (const detail of nonNullValue(response.validationDetail)) {
+            if (detail.result === "failure") {
+                throw new Error(detail.description);


Something we can deal with later: throwing an error here will tell our telemetry that this wizard actually failed to execute. But in reality the step succeeded in doing it's job, the linker is just invalid.

Maybe a UserCancelledError with a good cancellation step would be more accurate?

alexweininger · 2023-06-20T21:18:37Z

serviceconnector/src/validateServiceConnector/validateLinker.ts

+        sourceResourceUri: item?.id,
+    }
+
+    const title: string = vscode.l10n.t('Validate service connector');


nit: create the value where it's used instead of making a variable up here

alexweininger · 2023-06-20T21:19:27Z

serviceconnector/test/.eslintrc.js

+        "@microsoft/eslint-config-azuretools/test"
+    ],
+    "rules": {
+        "no-restricted-imports": "off"


Curious, where/what are we importing that we needed to turn this rule off?

I was mostly basing this off the other packages since they all turn this rule off. Based on some more investigation it seems like the base rule needs to be turned off since it can return incorrect errors. For more info here's the link.

alexweininger · 2023-06-20T21:20:38Z

Looking good. Just some final touches and we should be good to move forward and publish 🎸

MicroFish91 · 2023-06-20T22:17:42Z

Are you planning to add a README for this package?

alexweininger · 2023-06-23T18:30:38Z

serviceconnector/src/serviceConnectorClient.ts

+
+import { AzExtServiceClientCredentials } from "@microsoft/vscode-azext-utils";
+
+export async function createServiceConnectorClient(credentials: AzExtServiceClientCredentials) {


Ok so if we are going full LINKER then we should rename this file and method

alexweininger

🔗

Add create, delte, and validate service connector

d85ed79

alexweininger reviewed May 26, 2023

View reviewed changes

bwateratmsft reviewed May 30, 2023

View reviewed changes

serviceconnector/package.json Outdated Show resolved Hide resolved

bwateratmsft reviewed May 30, 2023

View reviewed changes

Requested changes

bdc16b6

motm32 marked this pull request as ready for review June 2, 2023 20:42

motm32 requested a review from a team as a code owner June 2, 2023 20:42

alexweininger reviewed Jun 2, 2023

View reviewed changes

MicroFish91 reviewed Jun 8, 2023

View reviewed changes

MicroFish91 mentioned this pull request Jun 8, 2023

Add Service Connector microsoft/vscode-azurecontainerapps#381

Open

nturinski reviewed Jun 8, 2023

View reviewed changes

More changes

bffb73c

nturinski reviewed Jun 12, 2023

View reviewed changes

alexweininger reviewed Jun 14, 2023

View reviewed changes

serviceconnector/package.json Outdated Show resolved Hide resolved

Update utils and other requested changes

8e28c97

motm32 added 6 commits June 16, 2023 10:22

Merge branch 'main' of github.com:microsoft/vscode-azuretools into me…

eed2d5e

…ganmott/serviceConnector

Add serviceconnector to filters

f55354b

Add license file and package script

bf420db

Remove pretest

acd583a

Add test folder

37de355

Remove fs-extra

c73c867

alexweininger reviewed Jun 20, 2023

View reviewed changes

motm32 added 2 commits June 23, 2023 11:17

Final touches

3b51fc1

Rename folders

c46f3f7

alexweininger reviewed Jun 23, 2023

View reviewed changes

Rename createServiceConnectorClient

41fefa2

alexweininger approved these changes Jun 23, 2023

View reviewed changes

motm32 changed the title ~~serviceconnector: Add Service Connector~~ serviceconnector: Add Service Connector 🔗 Jun 23, 2023

motm32 merged commit 10ece51 into main Jun 23, 2023

motm32 deleted the meganmott/serviceConnector branch June 23, 2023 18:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

serviceconnector: Add Service Connector 🔗 #1476

serviceconnector: Add Service Connector 🔗 #1476

motm32 commented May 25, 2023 •

edited

Loading

MicroFish91 Jun 8, 2023

MicroFish91 Jun 8, 2023

MicroFish91 commented Jun 8, 2023

motm32 commented Jun 8, 2023

nturinski Jun 8, 2023

motm32 Jun 8, 2023

nturinski Jun 12, 2023

nturinski Jun 12, 2023

nturinski Jun 12, 2023

nturinski Jun 12, 2023

nturinski Jun 12, 2023

bwateratmsft commented Jun 16, 2023 •

edited

Loading

alexweininger Jun 20, 2023

alexweininger Jun 20, 2023

alexweininger Jun 20, 2023

motm32 Jun 21, 2023

alexweininger Jun 20, 2023

alexweininger Jun 20, 2023

motm32 Jun 23, 2023

alexweininger Jun 23, 2023

alexweininger Jun 20, 2023

alexweininger Jun 20, 2023

alexweininger Jun 20, 2023

bwateratmsft Jun 21, 2023

alexweininger Jun 20, 2023

alexweininger Jun 20, 2023

motm32 Jun 21, 2023

alexweininger commented Jun 20, 2023

MicroFish91 commented Jun 20, 2023 •

edited

Loading

alexweininger Jun 23, 2023

alexweininger left a comment

		import { ISubscriptionActionContext } from "@microsoft/vscode-azext-utils";
		import { TargetServiceType } from "../../constants";


		import { AzExtServiceClientCredentials } from "@microsoft/vscode-azext-utils";

		export async function createServiceConnectorClient(credentials: AzExtServiceClientCredentials) {

serviceconnector: Add Service Connector 🔗 #1476

serviceconnector: Add Service Connector 🔗 #1476

Conversation

motm32 commented May 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MicroFish91 commented Jun 8, 2023

motm32 commented Jun 8, 2023

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

bwateratmsft commented Jun 16, 2023 • edited Loading

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

alexweininger commented Jun 20, 2023

MicroFish91 commented Jun 20, 2023 • edited Loading

Choose a reason for hiding this comment

alexweininger left a comment

Choose a reason for hiding this comment

motm32 commented May 25, 2023 •

edited

Loading

bwateratmsft commented Jun 16, 2023 •

edited

Loading

MicroFish91 commented Jun 20, 2023 •

edited

Loading