Skip to content

Better messaging on notebook fail #10056

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

Merged

Conversation

IanMatthewHuff
Copy link
Member

For #9904

Note. I was not able to repro the situation where I was failing and didn't see the missing dependencies popup. When I deleted or removed jupyter from the saved jupyter interpreter I always got the missing dependencies popup. So I added the target interpreter to that dialog (so the user can see what they are trying to use) and improved the help link.

  • Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR).
  • Title summarizes what is changing.
  • Has a news entry file (remember to thank yourself!).
  • Appropriate comments and documentation strings in the code.
  • Has sufficient logging.
  • Has telemetry for enhancements.
  • Unit tests & system/integration tests are added/updated.
  • Test plan is updated as appropriate.
  • package-lock.json has been regenerated by running npm install (if dependencies have changed).
  • The wiki is updated with any design decisions/details.

@@ -71,18 +71,18 @@ export class InteractivePanel extends React.Component<IInteractivePanelProps> {
<section id="main-panel-variable" aria-label={getLocString('DataScience.collapseVariableExplorerLabel', 'Variables')}>
{this.renderVariablePanel(this.props.baseTheme)}
</section>
<main id="main-panel-content" onClick={this.contentPanelClick} onScroll={this.handleScroll}>
<main id="main-panel-content" onScroll={this.handleScroll}>
Copy link
Member Author

Choose a reason for hiding this comment

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

Small change related to @DonJayamanne feedback on this PR:
#10029

I'm worried catching all clicks on the main panel is too aggressive. Don already noted that you now can't handle selecting sys info text to copy out. So instead moved it over to the footer. This had the nice benefit of greatly expanding the click area for the input box there. Previously you had to click just on the editor, which is close to the footer BG color. Now the entire footer can be clicked to focus the input.

Copy link

@rchiodo rchiodo left a comment

Choose a reason for hiding this comment

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

:shipit:

@codecov-io
Copy link

codecov-io commented Feb 11, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@8e205f0). Click here to learn what that means.
The diff coverage is 96.62%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #10056   +/-   ##
=========================================
  Coverage          ?   61.23%           
=========================================
  Files             ?      565           
  Lines             ?    30200           
  Branches          ?     4568           
=========================================
  Hits              ?    18494           
  Misses            ?    10673           
  Partials          ?     1033
Impacted Files Coverage Δ
src/datascience-ui/interactive-common/mainState.ts 61.97% <ø> (ø)
src/client/common/net/socket/socketServer.ts 15.38% <ø> (ø)
src/client/datascience/webViewHost.ts 60.83% <ø> (ø)
src/client/common/platform/fs-temp.ts 100% <ø> (ø)
...c/datascience-ui/react-common/settingsReactSide.ts 22.22% <ø> (ø)
src/client/common/logger.ts 79.62% <ø> (ø)
src/client/common/platform/types.ts 100% <ø> (ø)
src/client/datascience/types.ts 100% <ø> (ø)
src/client/common/platform/fileSystem.ts 80.97% <100%> (ø)
...interpreter/jupyterInterpreterDependencyService.ts 64.28% <100%> (ø)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8e205f0...ee0d6fa. Read the comment docs.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.4% 0.4% Duplication

@IanMatthewHuff IanMatthewHuff merged commit c8e7e5e into microsoft:master Feb 12, 2020
rchiodo pushed a commit that referenced this pull request Feb 15, 2020
rchiodo added a commit that referenced this pull request Feb 18, 2020
* Better messaging on notebook fail (#10056)

* Install jupyter instead of installing kernel spec (#10080)

* Install jupyter instead of installing kernel spec
For #10071

* Eliminate variable value when computing data frame info (#10081)

* Fix ndarray types to be viewable again (#10093)

* Eliminate variable value when computing data frame info

* Fix ndarrays to work again
Add test to make sure we don't regress this again

* Rchiodo/kernel telemetry (#10115)

* Add duration to select local/remote kernel

* Add notebook language telemetry

* Add news entries

* #9883 telemetry

* News entry

* Another spot for kernel spec failure

* Add telemetry on product install

* Fix install telemetry

* Undo launch.json change

* Handle other cases

* Better way to handle case

* Wrong event for jupyter install

* Fix unit tests

* Clear variables when restarting regardless if visible or not (#10117)

* Use different method for checking if kernelspec is available (#10114)

* Use different method for checking if kernelspec is available

* Fix unit tests

* More logging for kernelspec problems (#10132)

* More logging for kernelspec problems

* Actually capture the exception on the new code

* Not actually using output if first exception still there.

* Actually only return output on one of the expected calls.

* Fix nightly flake

* Check our saved jupyter interpreters before allowing any of them to be used as active interpreters (#10113)

* Update changelog and package.json

* Missing part of changelog

* Fix tests and linter problems

Co-authored-by: Ian Huff <[email protected]>
Co-authored-by: Don Jayamanne <[email protected]>
@lock lock bot locked as resolved and limited conversation to collaborators Feb 21, 2020
@IanMatthewHuff IanMatthewHuff deleted the dev/ianhu/betterNotebookFail branch July 9, 2020 22:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants