-
Notifications
You must be signed in to change notification settings - Fork 378
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Fixes various issue in the web app deployment script #4050
Conversation
if (-not $botPath) { | ||
# If don't provide bot path, then try to copy all dialogs except the runtime folder in parent folder to the publishing folder (bin\Realse\ Folder) | ||
$botPath = '..' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong default path. Must be on level higher.
} | ||
|
||
$botPath = $(Join-Path $botPath '*') | ||
Write-Host "Publishing dialogs from external bot project: $($botPath)" | ||
Copy-Item -Path (Get-Item -Path $botPath -Exclude ('runtime', 'generated')).FullName -Destination $remoteBotPath -Recurse -Force -Container | ||
|
||
# Try to get luis config from appsettings | ||
$settings = Get-Content $(Join-Path $projFolder appsettings.deployment.json) | ConvertFrom-Json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
appsettings.deployment.json has no effect to the published bot, should be appsettings.json
$luisSettings = $settings.luis | ||
|
||
if (-not $luisAuthoringKey) { | ||
$luisAuthoringKey = $luisSettings.authoringKey | ||
} | ||
|
||
if (-not $luisEndpointKey) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Secrets/key should not be published to the remote host -> should be configured via azure setting: luis__endpointkey
if (-not $luisAuthoringRegion) { | ||
$luisAuthoringRegion = $luisSettings.region | ||
} | ||
|
||
# set feature configuration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the local settings features if configured
# create generated folder if not exists | ||
if (!(Test-Path generated)) { | ||
New-Item -ItemType Directory -Force -Path generated | ||
} | ||
|
||
bf luis:build --luConfig $(Join-Path $remoteBotPath luconfig.json) --botName $name --authoringKey $luisAuthoringKey --dialog --out .\generated --suffix $customizedEnv -f --region $luisAuthoringRegion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the argument --dialog without value causes an error. In the latest branch it mus be crosstrained
|
||
bf luis:build --luConfig $(Join-Path $remoteBotPath luconfig.json) --botName $name --authoringKey $luisAuthoringKey --dialog --out .\generated --suffix $customizedEnv -f --region $luisAuthoringRegion | ||
|
||
bf luis:build --luConfig $(Join-Path $remoteBotPath luconfig.json) --botName $name --authoringKey $luisAuthoringKey --dialog crosstrained --out .\generated --suffix $environment -f --region $luisAuthoringRegion | ||
} | ||
else { | ||
Write-Host "bf luis:build does not exist, use the following command to install:" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Meanwhile there is a published version of the CLI
$settings | Add-Member -Type NoteProperty -Force -Name 'feature' -Value $featureConfig | ||
$settings | ConvertTo-Json -depth 100 | Out-File $(Join-Path $publishFolder appsettings.deployment.json) | ||
$settings | ConvertTo-Json -depth 100 | Out-File $settingsPath |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace settings in composer settings folder. The settings will only includes the luis app id mappings and feature configuration. All other settings must be configured via the azure web app settings
@ltwlf , we are deprecating this scripts, and use the scripts generated in your bot folder instead, could you please try that one? sorry for the confusion. |
I optimized the deploy script for CI/CD deployment and updated the readme accordingly. And I deleted the obsolete scripts. As soon this is merged I'm going to record a screen cast on how to do CI/CD pipelines with composer projects. |
deploy improvements fixes missing empty models; removes accidentally committed yarn.lock
@ltwlf please update/rebase your branches to represent the appropriate diff. Many commits outside this change are now included and the surface area is much too large. |
@cwhitten I rebased on the current main. The file changes are now correct again. |
…4050) * Fixes various issue in the web app deployment script deploy improvements fixes missing empty models; removes accidentally committed yarn.lock * Deletes obsolete scripts * Optimises deployment script for CI/CD pipeline; readme
Description
The azure webapp deployment script has various issues e.g. bf luis:build has wrong --dialog argument.
appsettings.deployment.json will never be used by the published bot and exposes local secrets to the remote folder.
Task Item
fixes #4049