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

GenerateToNotebook and Deploy buttons for Notebook Wizards #12656

Merged
merged 21 commits into from
Sep 30, 2020
Merged

Conversation

ranasaria
Copy link
Contributor

@ranasaria ranasaria commented Sep 29, 2020

This change adds a feature to support both ScriptToNotebook and Deploy buttons in a Notebook Wizard. The Deploy button now runs the notebook instead of executing the notebook using azdata in a new process.

Testing:

  1. End to end testing of the new feature.
  2. Extension tests.
  3. Ensuring no regression on BDC Wizard.

This PR fixes #12577

@coveralls
Copy link

coveralls commented Sep 29, 2020

Pull Request Test Coverage Report for Build 279519332

  • 13 of 65 (20.0%) changed or added relevant lines in 10 files are covered.
  • 3 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.02%) to 38.177%

Changes Missing Coverage Covered Lines Changed/Added Lines %
extensions/resource-deployment/src/services/resourceTypeService.ts 7 8 87.5%
extensions/resource-deployment/src/ui/deployAzureSQLVMWizard/deployAzureSQLVMWizard.ts 0 1 0.0%
extensions/resource-deployment/src/ui/deployClusterWizard/deployClusterWizard.ts 0 1 0.0%
extensions/resource-deployment/src/ui/deploymentInputDialog.ts 0 1 0.0%
extensions/resource-deployment/src/ui/wizardPageInfo.ts 1 6 16.67%
extensions/resource-deployment/src/services/notebookService.ts 0 6 0.0%
extensions/resource-deployment/src/ui/notebookWizard/notebookWizardPage.ts 0 7 0.0%
extensions/resource-deployment/src/ui/notebookWizard/notebookWizard.ts 1 16 6.25%
extensions/resource-deployment/src/ui/wizardBase.ts 1 16 6.25%
Files with Coverage Reduction New Missed Lines %
extensions/resource-deployment/src/ui/notebookWizard/notebookWizard.ts 1 9.88%
extensions/resource-deployment/src/ui/wizardBase.ts 2 9.52%
Totals Coverage Status
Change from base Build 279389110: -0.02%
Covered Lines: 19223
Relevant Lines: 45936

💛 - Coveralls

@ranasaria ranasaria requested review from alanrenmsft, aasimkhan30, abist and chlafreniere and removed request for abist September 29, 2020 16:33
taskName?: string;
type?: DeploymentType;
actionText?: string;
/**
* done button attributes.
Copy link
Contributor

Choose a reason for hiding this comment

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

extra spaces here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do not see the extra space.

pages: NotebookWizardPageInfo[]
}

export interface WizardInfoBase extends FieldInfoBase {
/**
* the taskName to use when running the notebook as a background task. The default value is the @see title of the wizard.
Copy link
Contributor

Choose a reason for hiding this comment

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

We can get rid of this now right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will check to ensure none of subclasses are using it.

@ranasaria ranasaria merged commit b8de69d into main Sep 30, 2020
@ranasaria ranasaria deleted the bug/12577-2 branch September 30, 2020 01:12
smartguest pushed a commit that referenced this pull request Oct 5, 2020
* enable userChooses how to run notebook

* arc ext changes

* nb fixes

* working version

* revert unneeded changes

* fix comments

* Update interfaces.ts

* fix comments

* fix comments

* fix comments

* runAllCells instead of background execute

* pr feedback

* PR feedback

* pr feedback

* arc ext changes for new WizardInfo syntax

* fix doc comments

* pr feedback
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.

Move Arc instance deployments to use Notebook wizards
4 participants