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

Refactor INotebook to be simpler #434

Closed
rchiodo opened this issue Nov 10, 2020 · 1 comment
Closed

Refactor INotebook to be simpler #434

rchiodo opened this issue Nov 10, 2020 · 1 comment
Labels
debt Code quality issues

Comments

@rchiodo
Copy link
Contributor

rchiodo commented Nov 10, 2020

Work Item

Split JupyterNotebook into separate classes

  • Execution wrapper (for everything - generic)
  • Kernel changing
  • Kernel initialization
  • Kernel creation
  • Connection
  • Kernel mapped to file URI

Determine if Live share is used or not

Original Complaint:

from here (http://pyretro.westus.azurecontainer.io:4000/#3b9f321e-9838-486e-a661-a5e96c46189f)

CellExecution and JupyterNotebook should be combined into a more generic algorithm (this is really jupyter exceution)
jupyterNotebook.ts should probably be renamed? That's where we handle IO pub messages, and I always have to dig around for that file when trying to debug kernel messages
Break INotebook into smaller components (componetise):

  • Separate class to communicate with kernel
  • Separate class to get active kernel & status of kernel & the like
  • Separate class to execute & manage execution state
  • Separate class to manage starting kernels
    JupyterNotebook should be split into multiple parts. Exposing the kernel, exposing the session, and execution. Exposing the kernel by itself would make the execution part easy to do alone
    Our initial "mega-classes" like INotebook Session are too big and expose too much.
    Break large classes into smaller components.
    Composition over inheritance & composition over god classes.
    Having one class that does everything makes it difficult to extend & re-purpose
@greazer greazer added debt Code quality issues webview-cleanup labels Aug 2, 2021
@IanMatthewHuff
Copy link
Member

Superseded by more recent #6950 and internal #191

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues
Projects
None yet
Development

No branches or pull requests

3 participants