fix for jupyterDebugger #7695
fix for jupyterDebugger #7695
Conversation
| '>=6.0.0', | ||
| notebook.getKernelConnection()?.interpreter | ||
| ); | ||
|
|
There was a problem hiding this comment.
Maybe put this into a common function somewhere?
There was a problem hiding this comment.
It would make it easier to handle the case where the ipykernel version is greater than 10.
There was a problem hiding this comment.
yees, I thought this was urgent but it isn't.
So I'll add it here.
There was a problem hiding this comment.
I agree this should be some common code,
There was a problem hiding this comment.
You can leave this here and fix in debt week or fix this now, i leave that upto you.
There was a problem hiding this comment.
If not doing now, please file an issue so we don't forget (to fix in debt week)
| const output = await kernel.executeHidden(code); | ||
|
|
||
| if (output[0].text) { | ||
| const majorVersion = Number(output[0].text.toString().charAt(0)); |
There was a problem hiding this comment.
There should be something already to parse semvers I think? Yeah it's an npm module for parsing them.
There was a problem hiding this comment.
Aren't python modules not strict on semver though? Lemme look that up quick. Might not be safe to use.
There was a problem hiding this comment.
Yes, that's correct
& I think looking at chat(0) is good enough (personally i think this is the best), better than unnecessarily pull in semver or other stuff, but we'll need to address the version hitting 10, at that point it till be 1. we could always do the necessary parsing in python code as well. Either or...something for debt week.
-split string to get more than 1 char of the major version
Codecov Report
@@ Coverage Diff @@
## main #7695 +/- ##
=====================================
- Coverage 68% 68% -1%
=====================================
Files 363 363
Lines 22590 22594 +4
Branches 3437 3438 +1
=====================================
- Hits 15542 15522 -20
- Misses 5716 5731 +15
- Partials 1332 1341 +9
|
Part 2 of #7689
package-lock.jsonhas been regenerated by runningnpm install(if dependencies have changed).