Skip to content
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

feat: Runtime refactor, new directory structure and Azure Function introduction phase 1 #2855

Merged
merged 24 commits into from
May 1, 2020

Conversation

carlosscastro
Copy link
Member

@carlosscastro carlosscastro commented May 1, 2020

Conceptual change of BotProject to bot runtime, since it makes it more universally clear and easily recognizable outside of the team. Improvements in c# runtime, refactors and laying the foundation for azure functions support, including alpha version of the azure functions runtime.

Folder Structure

As part of an effort to get Azure Functions integrated into composer I'm shuffling things around, and it marks a good time to re-think the folder structure for the bot runtime.

Proposed folder structure:

  • runtime/dotnet/ Dotnet root
  • runtime/dotnet/core/ Dotnet core shared libraries. Most of the dotnet code should be here.
  • runtime/dotnet/azurefunctions/
  • runtime/dotnet/azurewebapp/
  • runtime/node/core/
  • runtime/node/azurefunctions/
  • runtime/node/azurewebapp/
  • runtime/node/someothertechnology/

Note that we use dotnet in order to match with the botbuilder-* repos. We keep node since it really will host node bots for the first iteration. This also leaves the open slot for a js folder if / when we support browser js bots.

In a more generic way, the folder structure will be:

  • runtime/{language}/core/
  • runtime/{language}/{hostingTechnology1}/
  • runtime/{language}/{hostingTechnology2}/

One neat thing about this, is that the eject or publish commands will onlyu require to move the core library plus the selected integration library.

Directory structure proposal here: #2840

the build system forced me to add the line below as well :)

fixes #2840

Runtime Refactor

Separating the BotProject runtime into the following structure

  • Microsoft.BotFramework.Composer.Core: Core logic to be used generally by an assortment of runtimes. All code here is runtime agnostic.
  • Microsoft.BotFrameowrk.Composer.AzureWebApp: Thin head on top of the core library to support Azure WebApps. Only controllers and startup, calls the core library right away and no business logic here
  • Microsoft.BotFrameowrk.Composer.AzureFunctions: Thin head on top of the core library to support Azure Functions. Only HttpTriggers and FunctionsStartup, calls the core library right away and no business logic here.
  • Microsoft.BotFrameowrk.Composer.Tests: Test project for all c# code.

Composer server update

Updating the composer source to work with the new folder structure.

Next steps and phase 2

This change is not final in that 1) new functionality needs to be added and 2) the current functionality needs to be improved.

In the very short term (i.e. single digit days), the next steps are as follows

  • Test end to end for regressions on azure web app and add fixes as needed
  • Test Functions runtime with skills
  • Remove all hardcoded occurrences of azurewebapp and move to a variable model where each time we use azurewebapp, we have in scope an object with runtime metadata for the selected runtime, including the name (azurewebapp or azurefunctions, init command, run command and package command). This is a solid first step towards both progress and gaining understanding for the DCR [DCR] Interchangeable Runtimes: definition, requirements, commands and code impact #2852. This work to support functions will be a basic solution and not the final resolution of the DCR.

In the medium term, we could more refinements to this after proper planning and prioritization, such as

}


//[FunctionName("skills")]
Copy link
Member Author

Choose a reason for hiding this comment

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

This will come along in a second PR that adds function support.

@carlosscastro carlosscastro linked an issue May 1, 2020 that may be closed by this pull request
@carlosscastro carlosscastro marked this pull request as ready for review May 1, 2020 19:14
@@ -35,7 +35,7 @@ export interface BotProjectDeployConfig {
// The ARM template file path, default is 'DeploymentTemplates/template-with-preexisting-rg.json'
templatePath?: string;

// Dotnet project path, default is 'BotProject.csproj'
// Dotnet project path, default is 'Microsoft.BotFramework.Composer.WebApp.csproj'
Copy link
Member Author

Choose a reason for hiding this comment

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

All of these hardcoded paths are going away when we do the interchangeable runtimes DCR later on #2852

Copy link
Member

@cwhitten cwhitten left a comment

Choose a reason for hiding this comment

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

love this.

@carlosscastro what do we think about being more specific for the incoming JS stuff and call it "node" instead of "js"?

@@ -40,7 +40,7 @@
"start:client": "yarn workspace @bfc/client start",
"start:server": "yarn workspace @bfc/server start",
"start:server:dev": "yarn workspace @bfc/server start:dev",
"runtime": "cd ../BotProject/CSharp/ && dotnet build && dotnet run",
"runtime": "cd ../runtime/dotnet/azurewebapp && dotnet build && dotnet run",
Copy link
Member

Choose a reason for hiding this comment

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

good start here. i imagine we'll expand these script targets to configure the hosting platform..

runtime:azurewebapp, runtime:azurefunction

or accept a cli parameter, ex. "language", "host"
"runtime": "cd ../runtime/${language}/${host} .."`

but even then i'm not sure we'll be happy with it given that dotnet, node, python etc have disparate api's and it will require reworking this quite a bit.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this PR is just having what worked before, to work with the new structure. No new functionality. Having this be configurable is more of an R10 thing.

Copy link
Member Author

Choose a reason for hiding this comment

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

For composer to support interchangeable runtimes, composer needs to make no assumptions about the runtimes so we agree on that one. There will be significant reworking proportionally, but in practice there are 4 classes in composer that do mos tof the runtime interaction: plugins (azure + local ), assetManager and BotDeployer. It's reasonable to rework those plus command line tools, considering that in exchange we gain interchangeable runtimes if we engineer it right.

@@ -258,7 +262,9 @@ class AzurePublisher {
};

// append provision resource into file
const resourcePath = path.resolve(this.getProjectFolder(resourcekey), 'appsettings.deployment.json');
// TODO: here is where we configure the template for the runtime, and should be parameterized when we
Copy link
Member

Choose a reason for hiding this comment

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

yes exactly, we'll need to hoist this "appsettings" stuff out

Copy link
Member Author

Choose a reason for hiding this comment

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

#2852 In r10 we should get all these harcodings out

Composer/plugins/localPublish/src/index.ts Outdated Show resolved Hide resolved
@carlosscastro carlosscastro requested a review from yeze322 as a code owner May 1, 2020 19:27
@carlosscastro
Copy link
Member Author

carlosscastro commented May 1, 2020

@cwhitten calling it node seems reasonable. Specially if in the future we might have js, node and typescript (even if we don't implement it, somebody might, right?). I can add that and update DCRs.

@carlosscastro carlosscastro changed the title [Internal Dev PR] Runtime refactor, new directory structure and Azure Function introduction phase 1 feat: Runtime refactor, new directory structure and Azure Function introduction phase 1 May 1, 2020
@carlosscastro
Copy link
Member Author

love this.

@carlosscastro what do we think about being more specific for the incoming JS stuff and call it "node" instead of "js"?

Just changed folder name, readme files and DCRs to use the node name. Added an explanation in the DCR #2840

Note that we use dotnet in order to match with the botbuilder-* repos. We keep node since it really will host node bots for the first iteration. This also leaves the open slot for a js folder if / when we support browser js bots.

@cwhitten cwhitten merged commit 2694ffd into microsoft:wenyluo/azure May 1, 2020
cwhitten added a commit that referenced this pull request May 5, 2020
* split the configuration as object

* fix deployment script, change settings path

* fix bugs in deploy script and change credential type:

* finish gethistory, getstatus, update history and status after publish

* add some code comments for clarity
add many TODOs in code

* more code comments

* more code comments for clarity

* add login script and hash the config as folder name

* add bot deploy package build into lib build

* split the provision part out of composer

* fix luis appid unfound error

* fix typo

* polish

* use bot root to config the path of settings

* differ bot to different layer

* change the order of loading settings

* update schema

* change login and provision script, from save file to output to console

* add comment

* fix merge conflict

* format resulting profile

* add comment about security

* use Bearer token auth when doing zipdeploy
remove use of websiteclient

* move provision out of composer, and make sure it not depend on internal package

* add provision subfields schema

* use token replace the credential

* fix output of token

* change the error message when token expire

* add provision error details, add configurable

* update schema require and log message return

* fix parse

* fix template

* update schema and provision script

* fix provision luis configurable

* detail the token's error handle

* Add support for using the ejected runtime code instead of the built in template when publishing to Azure

* Make it optional to persist the history to disk.
Keep it in memory instead.

* move provision script into shared asset folder that is copied into all new projets
change hashing to include bot name to avoid possible overlap when using shared resources

* add to readme

* more readme

* more readme

* clean up tmp folder after publish completes or fails

* remove empty config in provision and appPassword in provision result, Change provision default to true

* add token error handle when zipdeploy fail

* improve output of provision script:
* add usage if missing parameters
* colorize and format output
* add some additional error formatting

* improve output

* remove webManagerClient and use token instead

* clean up the order of fields, remove unused fields from profile
update schema with full form

* use publisher description rather than package name

* feat: Runtime refactor, new directory structure and Azure Function introduction phase 1 (#2855)

* Runtime: new folder structure, refactor common c# code into core, create function runtime

* Remove deprecated Bot Project

* Runtime: Fix tests

* Update runtime code owners

* Runtime: Part 1 of updating composer server to honor new runtime paths

* Runtime: tweaks for local publish to run prior to merge

* Runtime: fixes post merge

* Update azure publish to use the azure web app template.

* Azure publish: update bot project deploy to new directory structure

* Fix codeowners alias mistake

* Revert "Merge branch 'master' of https://github.com/microsoft/BotFramework-Composer into wenyluo/azure"

This reverts commit 76251ec, reversing
changes made to 7d83253.

* WebApp + new runtime: deployment and local runtime all working

* Runtime: move nuget.config to dotnet runtime root

* Asset manager: add mock folder to reflect new runtime structure

* Local publish: remove unnecessary space

* Runtime: Add copyright header to all missing files

* Fix bad merge

* Runtime: rename js -> node and update readmes

* add an optional instructions field to the publish plugin so we ca nexplain how to get a config

* merge master

* fix linter warning

* handle errors in kill procss

* remove unnecessary dupe fields from schema

* fix for modified schema path

Co-authored-by: Wenyi Luo <wenyluo@microsoft.com>
Co-authored-by: Qi Kang <qika@microsoft.com>
Co-authored-by: Chris Whitten <christopher.whitten@microsoft.com>
Co-authored-by: Carlos Castro <carlosscastro@users.noreply.github.com>
cwhitten added a commit that referenced this pull request May 12, 2020
…2923)

* split the configuration as object

* fix deployment script, change settings path

* fix bugs in deploy script and change credential type:

* finish gethistory, getstatus, update history and status after publish

* add some code comments for clarity
add many TODOs in code

* more code comments

* more code comments for clarity

* add login script and hash the config as folder name

* add bot deploy package build into lib build

* split the provision part out of composer

* fix luis appid unfound error

* fix typo

* polish

* use bot root to config the path of settings

* differ bot to different layer

* change the order of loading settings

* update schema

* change login and provision script, from save file to output to console

* add comment

* fix merge conflict

* format resulting profile

* add comment about security

* use Bearer token auth when doing zipdeploy
remove use of websiteclient

* move provision out of composer, and make sure it not depend on internal package

* add provision subfields schema

* use token replace the credential

* fix output of token

* change the error message when token expire

* add provision error details, add configurable

* update schema require and log message return

* fix parse

* fix template

* update schema and provision script

* fix provision luis configurable

* detail the token's error handle

* Add support for using the ejected runtime code instead of the built in template when publishing to Azure

* Make it optional to persist the history to disk.
Keep it in memory instead.

* move provision script into shared asset folder that is copied into all new projets
change hashing to include bot name to avoid possible overlap when using shared resources

* add to readme

* more readme

* more readme

* clean up tmp folder after publish completes or fails

* remove empty config in provision and appPassword in provision result, Change provision default to true

* add token error handle when zipdeploy fail

* improve output of provision script:
* add usage if missing parameters
* colorize and format output
* add some additional error formatting

* improve output

* remove webManagerClient and use token instead

* clean up the order of fields, remove unused fields from profile
update schema with full form

* use publisher description rather than package name

* feat: Runtime refactor, new directory structure and Azure Function introduction phase 1 (#2855)

* Runtime: new folder structure, refactor common c# code into core, create function runtime

* Remove deprecated Bot Project

* Runtime: Fix tests

* Update runtime code owners

* Runtime: Part 1 of updating composer server to honor new runtime paths

* Runtime: tweaks for local publish to run prior to merge

* Runtime: fixes post merge

* Update azure publish to use the azure web app template.

* Azure publish: update bot project deploy to new directory structure

* Fix codeowners alias mistake

* Revert "Merge branch 'master' of https://github.com/microsoft/BotFramework-Composer into wenyluo/azure"

This reverts commit 76251ec, reversing
changes made to 7d83253.

* WebApp + new runtime: deployment and local runtime all working

* Runtime: move nuget.config to dotnet runtime root

* Asset manager: add mock folder to reflect new runtime structure

* Local publish: remove unnecessary space

* Runtime: Add copyright header to all missing files

* Fix bad merge

* Runtime: rename js -> node and update readmes

* add an optional instructions field to the publish plugin so we ca nexplain how to get a config

* merge master

* fix linter warning

* create space for azure functions publish

* Runtime + Deployment: Azure function support (preview version)

* Deploy plugin: Scripts package lock

* Remove package lock to make build happy

* Plugins: attempt non-concurrent yarn install to avoid yarn known integrity issue

* Plugin provision script: Remove functions script, add param and unify into single script

* Azure functions runtime: Correct configuration precedence

* latest schema from master

* little deltas

* Functions: Add declarative component registration

Co-authored-by: Wenyi Luo <wenyluo@microsoft.com>
Co-authored-by: Qi Kang <qika@microsoft.com>
Co-authored-by: Ben Brown <benbro@microsoft.com>
Co-authored-by: Chris Whitten <christopher.whitten@microsoft.com>
benbrown pushed a commit to benbrown/BotFramework-Composer that referenced this pull request May 24, 2021
…troduction phase 1 (microsoft#2855)

* Runtime: new folder structure, refactor common c# code into core, create function runtime

* Remove deprecated Bot Project

* Runtime: Fix tests

* Update runtime code owners

* Runtime: Part 1 of updating composer server to honor new runtime paths

* Runtime: tweaks for local publish to run prior to merge

* Runtime: fixes post merge

* Update azure publish to use the azure web app template.

* Azure publish: update bot project deploy to new directory structure

* Fix codeowners alias mistake

* Revert "Merge branch 'master' of https://github.com/microsoft/BotFramework-Composer into wenyluo/azure"

This reverts commit 76251ec, reversing
changes made to 7d83253.

* WebApp + new runtime: deployment and local runtime all working

* Runtime: move nuget.config to dotnet runtime root

* Asset manager: add mock folder to reflect new runtime structure

* Local publish: remove unnecessary space

* Runtime: Add copyright header to all missing files

* Fix bad merge

* Runtime: rename js -> node and update readmes
benbrown pushed a commit that referenced this pull request Jun 11, 2021
…troduction phase 1 (#2855)

* Runtime: new folder structure, refactor common c# code into core, create function runtime

* Remove deprecated Bot Project

* Runtime: Fix tests

* Update runtime code owners

* Runtime: Part 1 of updating composer server to honor new runtime paths

* Runtime: tweaks for local publish to run prior to merge

* Runtime: fixes post merge

* Update azure publish to use the azure web app template.

* Azure publish: update bot project deploy to new directory structure

* Fix codeowners alias mistake

* Revert "Merge branch 'master' of https://github.com/microsoft/BotFramework-Composer into wenyluo/azure"

This reverts commit af077e248e47e32750629dee7b9b8c8c6e98d84a, reversing
changes made to e24071d0ddbce5554d498c1ba34eb18a93b8948d.

* WebApp + new runtime: deployment and local runtime all working

* Runtime: move nuget.config to dotnet runtime root

* Asset manager: add mock folder to reflect new runtime structure

* Local publish: remove unnecessary space

* Runtime: Add copyright header to all missing files

* Fix bad merge

* Runtime: rename js -> node and update readmes
lei9444 pushed a commit to lei9444/BotFramework-Composer-1 that referenced this pull request Jun 15, 2021
* split the configuration as object

* fix deployment script, change settings path

* fix bugs in deploy script and change credential type:

* finish gethistory, getstatus, update history and status after publish

* add some code comments for clarity
add many TODOs in code

* more code comments

* more code comments for clarity

* add login script and hash the config as folder name

* add bot deploy package build into lib build

* split the provision part out of composer

* fix luis appid unfound error

* fix typo

* polish

* use bot root to config the path of settings

* differ bot to different layer

* change the order of loading settings

* update schema

* change login and provision script, from save file to output to console

* add comment

* fix merge conflict

* format resulting profile

* add comment about security

* use Bearer token auth when doing zipdeploy
remove use of websiteclient

* move provision out of composer, and make sure it not depend on internal package

* add provision subfields schema

* use token replace the credential

* fix output of token

* change the error message when token expire

* add provision error details, add configurable

* update schema require and log message return

* fix parse

* fix template

* update schema and provision script

* fix provision luis configurable

* detail the token's error handle

* Add support for using the ejected runtime code instead of the built in template when publishing to Azure

* Make it optional to persist the history to disk.
Keep it in memory instead.

* move provision script into shared asset folder that is copied into all new projets
change hashing to include bot name to avoid possible overlap when using shared resources

* add to readme

* more readme

* more readme

* clean up tmp folder after publish completes or fails

* remove empty config in provision and appPassword in provision result, Change provision default to true

* add token error handle when zipdeploy fail

* improve output of provision script:
* add usage if missing parameters
* colorize and format output
* add some additional error formatting

* improve output

* remove webManagerClient and use token instead

* clean up the order of fields, remove unused fields from profile
update schema with full form

* use publisher description rather than package name

* feat: Runtime refactor, new directory structure and Azure Function introduction phase 1 (microsoft#2855)

* Runtime: new folder structure, refactor common c# code into core, create function runtime

* Remove deprecated Bot Project

* Runtime: Fix tests

* Update runtime code owners

* Runtime: Part 1 of updating composer server to honor new runtime paths

* Runtime: tweaks for local publish to run prior to merge

* Runtime: fixes post merge

* Update azure publish to use the azure web app template.

* Azure publish: update bot project deploy to new directory structure

* Fix codeowners alias mistake

* Revert "Merge branch 'master' of https://github.com/microsoft/BotFramework-Composer into wenyluo/azure"

This reverts commit af077e248e47e32750629dee7b9b8c8c6e98d84a, reversing
changes made to e24071d0ddbce5554d498c1ba34eb18a93b8948d.

* WebApp + new runtime: deployment and local runtime all working

* Runtime: move nuget.config to dotnet runtime root

* Asset manager: add mock folder to reflect new runtime structure

* Local publish: remove unnecessary space

* Runtime: Add copyright header to all missing files

* Fix bad merge

* Runtime: rename js -> node and update readmes

* add an optional instructions field to the publish plugin so we ca nexplain how to get a config

* merge master

* fix linter warning

* handle errors in kill procss

* remove unnecessary dupe fields from schema

* fix for modified schema path

Co-authored-by: Wenyi Luo <wenyluo@microsoft.com>
Co-authored-by: Qi Kang <qika@microsoft.com>
Co-authored-by: Chris Whitten <christopher.whitten@microsoft.com>
Co-authored-by: Carlos Castro <carlosscastro@users.noreply.github.com>
lei9444 pushed a commit to lei9444/BotFramework-Composer-1 that referenced this pull request Jun 15, 2021
…icrosoft#2923)

* split the configuration as object

* fix deployment script, change settings path

* fix bugs in deploy script and change credential type:

* finish gethistory, getstatus, update history and status after publish

* add some code comments for clarity
add many TODOs in code

* more code comments

* more code comments for clarity

* add login script and hash the config as folder name

* add bot deploy package build into lib build

* split the provision part out of composer

* fix luis appid unfound error

* fix typo

* polish

* use bot root to config the path of settings

* differ bot to different layer

* change the order of loading settings

* update schema

* change login and provision script, from save file to output to console

* add comment

* fix merge conflict

* format resulting profile

* add comment about security

* use Bearer token auth when doing zipdeploy
remove use of websiteclient

* move provision out of composer, and make sure it not depend on internal package

* add provision subfields schema

* use token replace the credential

* fix output of token

* change the error message when token expire

* add provision error details, add configurable

* update schema require and log message return

* fix parse

* fix template

* update schema and provision script

* fix provision luis configurable

* detail the token's error handle

* Add support for using the ejected runtime code instead of the built in template when publishing to Azure

* Make it optional to persist the history to disk.
Keep it in memory instead.

* move provision script into shared asset folder that is copied into all new projets
change hashing to include bot name to avoid possible overlap when using shared resources

* add to readme

* more readme

* more readme

* clean up tmp folder after publish completes or fails

* remove empty config in provision and appPassword in provision result, Change provision default to true

* add token error handle when zipdeploy fail

* improve output of provision script:
* add usage if missing parameters
* colorize and format output
* add some additional error formatting

* improve output

* remove webManagerClient and use token instead

* clean up the order of fields, remove unused fields from profile
update schema with full form

* use publisher description rather than package name

* feat: Runtime refactor, new directory structure and Azure Function introduction phase 1 (microsoft#2855)

* Runtime: new folder structure, refactor common c# code into core, create function runtime

* Remove deprecated Bot Project

* Runtime: Fix tests

* Update runtime code owners

* Runtime: Part 1 of updating composer server to honor new runtime paths

* Runtime: tweaks for local publish to run prior to merge

* Runtime: fixes post merge

* Update azure publish to use the azure web app template.

* Azure publish: update bot project deploy to new directory structure

* Fix codeowners alias mistake

* Revert "Merge branch 'master' of https://github.com/microsoft/BotFramework-Composer into wenyluo/azure"

This reverts commit af077e248e47e32750629dee7b9b8c8c6e98d84a, reversing
changes made to e24071d0ddbce5554d498c1ba34eb18a93b8948d.

* WebApp + new runtime: deployment and local runtime all working

* Runtime: move nuget.config to dotnet runtime root

* Asset manager: add mock folder to reflect new runtime structure

* Local publish: remove unnecessary space

* Runtime: Add copyright header to all missing files

* Fix bad merge

* Runtime: rename js -> node and update readmes

* add an optional instructions field to the publish plugin so we ca nexplain how to get a config

* merge master

* fix linter warning

* create space for azure functions publish

* Runtime + Deployment: Azure function support (preview version)

* Deploy plugin: Scripts package lock

* Remove package lock to make build happy

* Plugins: attempt non-concurrent yarn install to avoid yarn known integrity issue

* Plugin provision script: Remove functions script, add param and unify into single script

* Azure functions runtime: Correct configuration precedence

* latest schema from master

* little deltas

* Functions: Add declarative component registration

Co-authored-by: Wenyi Luo <wenyluo@microsoft.com>
Co-authored-by: Qi Kang <qika@microsoft.com>
Co-authored-by: Ben Brown <benbro@microsoft.com>
Co-authored-by: Chris Whitten <christopher.whitten@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DCR] Bot runtime directory structure changes
3 participants