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

Better logs when interpreter does not match kernel #5510

Merged
merged 1 commit into from
Apr 14, 2021

Conversation

DonJayamanne
Copy link
Contributor

For #5509

@DonJayamanne DonJayamanne requested a review from a team as a code owner April 13, 2021 16:56
match: match ? 'true' : 'false',
kernelConnectionType: kernelConnection.kind
});
trackKernelResourceInformation(Uri.file(file), { interpreterMatchesKernel: match });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

With the existing infrastructure we can tell:

  • If its a notebook vs interactive window
  • If its custom vs native notebook
  • If user ran the cell vs auto start
    All three are crucial and this wasn't easy to get with the previous event.

Removing the old item, once we add this, we can deprecate the old event.

@codecov-io
Copy link

Codecov Report

Merging #5510 (6ab967a) into main (f536e12) will decrease coverage by 0%.
The diff coverage is 100%.

@@          Coverage Diff          @@
##            main   #5510   +/-   ##
=====================================
- Coverage     73%     72%   -1%     
=====================================
  Files        401     401           
  Lines      26599   26599           
  Branches    3884    3884           
=====================================
- Hits       19418   19396   -22     
- Misses      5582    5585    +3     
- Partials    1599    1618   +19     
Impacted Files Coverage Δ
src/client/datascience/constants.ts 99% <ø> (-1%) ⬇️
src/client/datascience/telemetry/telemetry.ts 82% <ø> (+<1%) ⬆️
src/client/telemetry/index.ts 63% <ø> (ø)
src/client/datascience/jupyter/kernels/helpers.ts 75% <100%> (-2%) ⬇️
...ent/datascience/jupyter/jupyterExecutionFactory.ts 66% <0%> (-6%) ⬇️
src/client/datascience/jupyter/jupyterExecution.ts 64% <0%> (-5%) ⬇️
src/client/datascience/jupyter/jupyterServer.ts 72% <0%> (-5%) ⬇️
src/client/datascience/jupyter/serverPreload.ts 78% <0%> (-3%) ⬇️
...lient/datascience/jupyter/jupyterSessionManager.ts 62% <0%> (-3%) ⬇️
...er/jupyterInterpreterSubCommandExecutionService.ts 85% <0%> (-2%) ⬇️
... and 14 more

@DonJayamanne DonJayamanne force-pushed the addMoreKernelTelemetry branch from 6ab967a to a876c4b Compare April 13, 2021 19:36
@DonJayamanne
Copy link
Contributor Author

@IanMatthewHuff @DavidKutu I reverted the code that removed the old telemetry, will need that to compare in the future (see if we're getting better)

@DonJayamanne DonJayamanne merged commit 967709c into main Apr 14, 2021
@DonJayamanne DonJayamanne deleted the addMoreKernelTelemetry branch April 14, 2021 18:04
@DonJayamanne DonJayamanne mentioned this pull request Apr 19, 2021
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.

5 participants