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.

Sign up for GitHub

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

Jump to bottom

feat: bf-orchestrator lib integration #4384

Merged
merged 27 commits into from
Oct 26, 2020
Merged

feat: bf-orchestrator lib integration #4384

merged 27 commits into from
Oct 26, 2020

Conversation

taicchoumsft
Copy link
Contributor

@taicchoumsft taicchoumsft commented Oct 15, 2020 •
edited by boydc2014
Loading

Description

Integrate the Orchestrator nodejs library, which provides interfaces for working with the upcoming transformer-based Orchestrator Recognizer.

This PR exposes these interfaces for upper layers in Composer to use:

  • List available NLR models
  • Download an NLR model, and
  • Build a snapshot

Task Item

fixes #4132

@taicchoumsft taicchoumsft self-assigned this Oct 15, 2020
@taicchoumsft taicchoumsft linked an issue Oct 15, 2020 that may be closed by this pull request
Bf-orchestrator lib integration #4132
Closed
@taicchoumsft taicchoumsft changed the title Tachou/orchestratorlib feat: bf-orchestrator lib integration Oct 15, 2020
@boydc2014
Copy link
Contributor

@lei9444 can you help check this one?

tsuwandy
tsuwandy previously approved these changes Oct 15, 2020
View reviewed changes
Composer/packages/server/package.json Outdated Show resolved Hide resolved
boydc2014
boydc2014 previously approved these changes Oct 20, 2020
View reviewed changes
@taicchoumsft
Copy link
Contributor Author

taicchoumsft commented Oct 21, 2020 •
edited
Loading

@taicchoumsft this looks good now, can you address the conflict?

Hi @boydc2014, I've addressed the conflict in orchestrator-core, verified that the orchestrator UTs are running correctly in our Linux based CI pipeline, and then for developer convenience (because we d/l a large model every time), disabled the orchestrator tests again until Orchestrator team provides a local model.

@taicchoumsft taicchoumsft reopened this Oct 21, 2020
@boydc2014
Copy link
Contributor

boydc2014 commented Oct 22, 2020 •
edited
Loading

@taicchoumsft this looks good now, can you address the conflict?

Hi @boydc2014, I've addressed the conflict in orchestrator-core, verified that the orchestrator UTs are running correctly in our Linux based CI pipeline, and then for developer convenience (because we d/l a large model every time), disabled the orchestrator tests again until Orchestrator team provides a local model.

Cool, it's ready to take once the CI is green, i'm re-running the Docker build validation now.

@cdonke cdonke mentioned this pull request Oct 23, 2020
Closed
@cdonke
Copy link
Contributor

cdonke commented Oct 25, 2020

Changed the dockerfile base image to ubuntu on #4516 .

Hope it helps.

@boydc2014 boydc2014 merged commit 006a589 into main Oct 26, 2020
@boydc2014 boydc2014 deleted the tachou/orchestratorlib branch October 26, 2020 06:52
@lei9444 lei9444 mentioned this pull request Oct 26, 2020
Closed
8 tasks
taicchoumsft added a commit that referenced this pull request Oct 28, 2020
@taicchoumsft
Revert "feat: bf-orchestrator lib integration (#4384)"
212a58c 
This reverts commit 006a589.
@cwhitten cwhitten mentioned this pull request Nov 13, 2020
Merged
@taicchoumsft taicchoumsft mentioned this pull request Nov 18, 2020
Merged
lei9444 pushed a commit to lei9444/BotFramework-Composer-1 that referenced this pull request Jun 15, 2021
@taicchoumsft @boydc2014
feat: bf-orchestrator lib integration (microsoft#4384)
178eb9a 
* Add bf-orchestrator dependency

* Basic API and FT

* Add test to ignore list for now

* Fix linter issues

* Update orchestrator-core to pick up Linux fixes

* Fixes after latest merge

* Attempt to run orchestrator tests on CI

* Disable test again local model is available

* Try to bring glic into alpine-linux

* Revert "Try to bring glic into alpine-linux"

This reverts commit 4a111c45e7e98cdd6b54cd89c563cf79cada2c1f.

* Try switching to buster for Orchestrator

* lockdown orchestrator version due to API change

* Add libgomp dep for orchestrator

Co-authored-by: Dong Lei <donglei@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@feich-ms feich-ms feich-ms left review comments

@boydc2014 boydc2014 boydc2014 approved these changes

@tsuwandy tsuwandy tsuwandy left review comments

@cwhitten cwhitten Awaiting requested review from cwhitten

@a-b-r-o-w-n a-b-r-o-w-n Awaiting requested review from a-b-r-o-w-n

@daveta daveta Awaiting requested review from daveta

@yeze322 yeze322 Awaiting requested review from yeze322

@srinaath srinaath Awaiting requested review from srinaath

@zhixzhan zhixzhan Awaiting requested review from zhixzhan

@beyackle beyackle Awaiting requested review from beyackle

@GeoffCoxMSFT GeoffCoxMSFT Awaiting requested review from GeoffCoxMSFT

@hatpick hatpick Awaiting requested review from hatpick

@tonyanziano tonyanziano Awaiting requested review from tonyanziano

Assignees

@taicchoumsft taicchoumsft

Labels
Team: Platform
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bf-orchestrator lib integration
6 participants
@taicchoumsft @boydc2014 @coveralls @cdonke @tsuwandy @feich-ms