Skip to content

Use jupyer-kernel-gateway for ipython executor#1748

Merged
sonichi merged 25 commits intomicrosoft:mainfrom
jackgerrits:jupyter_client
Feb 23, 2024
Merged

Use jupyer-kernel-gateway for ipython executor#1748
sonichi merged 25 commits intomicrosoft:mainfrom
jackgerrits:jupyter_client

Conversation

@jackgerrits
Copy link
Copy Markdown
Contributor

Why are these changes needed?

This uses the jupyter-kernel-gateway to manage kernels for the ipython based executor. This is desirable as it allows us to have a common interface to interact with kernels irrespective of where they are executing. In this PR I migrate the embedded executor. I will follow this up with a docker container based version.

Related issue number

#1421

Checks

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Feb 21, 2024

Codecov Report

Attention: Patch coverage is 13.86139% with 261 lines in your changes are missing coverage. Please review.

Project coverage is 47.95%. Comparing base (ac15996) to head (4543624).

Files Patch % Lines
autogen/coding/jupyter/jupyter_client.py 10.09% 95 Missing and 3 partials ⚠️
autogen/coding/jupyter_code_executor.py 12.37% 85 Missing ⚠️
autogen/coding/jupyter/local_jupyter_server.py 0.00% 69 Missing and 1 partial ⚠️
autogen/coding/factory.py 0.00% 2 Missing and 1 partial ⚠️
autogen/coding/embedded_ipython_code_executor.py 60.00% 2 Missing ⚠️
autogen/coding/jupyter/__init__.py 50.00% 2 Missing ⚠️
autogen/coding/jupyter/base.py 92.30% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1748      +/-   ##
==========================================
+ Coverage   39.13%   47.95%   +8.82%     
==========================================
  Files          57       62       +5     
  Lines        6174     6473     +299     
  Branches     1384     1554     +170     
==========================================
+ Hits         2416     3104     +688     
+ Misses       3564     3118     -446     
- Partials      194      251      +57     
Flag Coverage Δ
unittests 47.87% <13.86%> (+8.74%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jackgerrits
Copy link
Copy Markdown
Contributor Author

Most of these change should be covered by the existing tests - but I don't think they are running since the dependencies are missing on the build machines

@ekzhu ekzhu self-requested a review February 21, 2024 17:33
@ekzhu
Copy link
Copy Markdown
Contributor

ekzhu commented Feb 22, 2024

Just a nit. Approved. Let's open an issue to track the upstream bug fix to the jupyter gateway client.

@jackgerrits
Copy link
Copy Markdown
Contributor Author

Upstream bug fix being tracked here #1763

@gagb
Copy link
Copy Markdown
Collaborator

gagb commented Feb 22, 2024

I am very confused how this PR affects me as an end user? Can you please provide examples.

@jackgerrits
Copy link
Copy Markdown
Contributor Author

This is part of a larger whole (including a docker based jupyter server), examples and docs will come with a later PR in the series

Copy link
Copy Markdown
Collaborator

@gagb gagb left a comment

Choose a reason for hiding this comment

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

This PR works. Lets improve the syntax of updating system messages via introductions in a different PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-execution execute generated code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants