feat: add js version deployment script #2463
Conversation
There was a problem hiding this comment.
@zidaneymar we should move this module into the /Composer application directory. This program will not be executed as a CLI command. It will be imported into the Composer application runtime and used in the web-app.
Because of this, Dong's comments around console.log are appropriate. The module needs an interface that communicates status and error state to the caller, but how this status is communicated to the user is not a concern of this module. Does this make sense?
I'm tagging @carlosscastro to also look at the implementation for additional suggestions, as he has experiencing building workflow against these apis.
@zidaneymar please update the description of the pull request to describe the api and intended use, so reviewers can better understand the structure and motivations.
|
P0 to remove console.logs and have well defined error conditions that are bubbled to library callers |
|
The Azure library usage looks good, great that you are using ARM client libraries. |
|
|
||
| private async dotnetPublish(publishFolder: string, projFolder: string, botPath?: string) { | ||
| const projectPath = path.join(projFolder, 'BotProject.csproj') | ||
| await exec(`dotnet publish ${projectPath} -c release -o ${publishFolder} -v q`) |
There was a problem hiding this comment.
To be consistent, you could consider using ARM to deploy rather than dotnet publish. The nice thing is that web app zip deploy through ARM will be useful the day we have other, non-dotnet languages, since it just requires a zip file.
Here more info: https://docs.microsoft.com/en-us/azure/app-service/deploy-zip
There was a problem hiding this comment.
To be consistent, you could consider using ARM to deploy rather than dotnet publish. The nice thing is that web app zip deploy through ARM will be useful the day we have other, non-dotnet languages, since it just requires a zip file.
Here more info: https://docs.microsoft.com/en-us/azure/app-service/deploy-zip
hi carloss, I use dotnet publish to produce an executable dotnet project, and then use the zip deploy you mentioned to publish that folder
| await fs.writeJson(deploymentSettingsPath, settings) | ||
| const token = await this.creds.getToken() | ||
|
|
||
| const getAccountUri = `${luisEndpoint}/luis/api/v2.0/azureaccounts` |
There was a problem hiding this comment.
Is there a luis client that we could use for this instead of crafting urls and requests manually? So that if the luis API changes, the client does the changes for us :)
There was a problem hiding this comment.
Is there a luis client that we could use for this instead of crafting urls and requests manually? So that if the luis API changes, the client does the changes for us :)
yes, I should use that client, and indeed there is an azure luis client inside the @azure-js-sdk, but it also have some issues, I have tried to use that client for luis accounts authoring but failed, so I turned to use the straight forward http request way..
|
@zidaneymar can you please address conflicts? |
resolved |
| private remoteBotPath: string; | ||
| private logger: (string) => any; | ||
|
|
||
| private readonly tenantId = '72f988bf-86f1-41af-91ab-2d7cd011db47'; |
There was a problem hiding this comment.
whose tenant id is this? It's used globally across all instances of this library?
There was a problem hiding this comment.
@cwhitten
it belongs to microsoft organization, and we need this tenant id to initialize a ms graph client to create an app registration based on the password provided by user.
const graphClient = new GraphRbacManagementClient(credsForGraph, this.tenantId, {
baseUri: 'https://graph.windows.net',
});
for app registration, azure js sdk also provide an options for whether this application is available to other tenants,
/**
* Whether the application is available to other tenants.
*/
availableToOtherTenants?: boolean;
| } from '@azure/arm-resources/esm/models'; | ||
| import { GraphRbacManagementClient } from '@azure/graph'; | ||
| import * as msRestNodeAuth from '@azure/ms-rest-nodeauth'; | ||
| import * as fs from 'fs-extra'; |
There was a problem hiding this comment.
I'm concerned about this library being bound to being used on the filesystem. We should hook into the storage layer of Composer to reuse the serialization configurations when writing .dialog, .lu, .lg, so when someone has CosmosDB as its storage layer, this library can still service their Azure deployment needs.
@benbrown @carlosscastro Looking for your suggestions on priority. Do we P0 or P1 this? The work to pull this out is non-trivial and could take awhile. Perhaps we land Publishing & Runtime E2E and revisit.
There was a problem hiding this comment.
i need to get deeper into how all of this works, but as it is, we pull stuff out of storage and onto disk before doing the deploy -- so in that sense, it doesn't really matter.
* js deployment init * add empty lu models check * rename * fix comments, change directory under Composer, make script configurable * fix yarn.lock * add logger status Co-authored-by: Qi Kang <qika@microsoft.com> Co-authored-by: Lu Han <32191031+luhan2017@users.noreply.github.com> Co-authored-by: Chris Whitten <christopher.whitten@microsoft.com>
Description
add js version deploy script, same as the powershell one.
closes #2461
Task Item
closes #2461
part of the azure deploy plugin
Screenshots