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

Feature: Add ability to use a separate python environment in local executor #2615

Merged
merged 16 commits into from
May 11, 2024

Conversation

Gr3atWh173
Copy link
Collaborator

@Gr3atWh173 Gr3atWh173 commented May 7, 2024

Why are these changes needed?

To execute code inside a python virtual environment so that the base environment isn't polluted by unwanted dependencies.

Related issue number

Solves #2613

Checks

@Gr3atWh173
Copy link
Collaborator Author

@microsoft-github-policy-service agree

@Gr3atWh173 Gr3atWh173 changed the title Feature/virtualenv in local executor Feature: Add ability to use a separate python environment in local executor May 7, 2024
@Gr3atWh173 Gr3atWh173 marked this pull request as draft May 7, 2024 17:40
Copy link
Collaborator

@ekzhu ekzhu left a comment

Choose a reason for hiding this comment

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

Shall we update the execution_policies argument so it can supply the executable binary for Python?

@ekzhu ekzhu requested a review from jackgerrits May 7, 2024 23:21
@Gr3atWh173
Copy link
Collaborator Author

Right now an execution policy is defined as a str to bool mapping. I think it will be a breaking change to change its type. IMO a better solution is to pass a environment context object to the executor. It won't break existing code that depends on Autogen and solve the issues I'm having with handling different venv directory structures for Windows and Unix systems.

@codecov-commenter
Copy link

Codecov Report

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

Project coverage is 15.75%. Comparing base (372ac1e) to head (1dff475).
Report is 5 commits behind head on main.

Files Patch % Lines
autogen/coding/local_commandline_code_executor.py 16.66% 10 Missing ⚠️
autogen/code_utils.py 37.50% 5 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2615       +/-   ##
===========================================
- Coverage   33.11%   15.75%   -17.37%     
===========================================
  Files          86       86               
  Lines        9108     9128       +20     
  Branches     1938     2093      +155     
===========================================
- Hits         3016     1438     -1578     
- Misses       5837     7642     +1805     
+ Partials      255       48      -207     
Flag Coverage Δ
unittests 15.75% <25.00%> (-17.37%) ⬇️

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.

@Gr3atWh173 Gr3atWh173 marked this pull request as ready for review May 9, 2024 03:42
@Gr3atWh173
Copy link
Collaborator Author

Needed to add a special case for Windows because of how CreateProcessW() works internally. Here's a good explainer on why adding the interpreter to PATH was not working: python/cpython#105889 (comment)

Copy link
Collaborator

@ekzhu ekzhu left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! A few questions:

  1. Does this only work with Python virtual environment not Anaconda?
  2. Can you add a code block example in the doc string for how to use the virtual_env_context argument?
  3. Can you update the documentation in website/docs/topics/code-execution/cli-code-executor.ipynb for an example on how to use this?

@ekzhu ekzhu added the code-execution execute generated code label May 11, 2024
@Gr3atWh173
Copy link
Collaborator Author

I don't think these changes will work with Anaconda since it doesn't use venv to manage environments IIRC

@ekzhu ekzhu added this pull request to the merge queue May 11, 2024
@ekzhu
Copy link
Collaborator

ekzhu commented May 11, 2024

Thanks @Gr3atWh173 can you add your discord handle so we can connect with you on AutoGen's discord server: #2310

Merged via the queue into microsoft:main with commit 60c6658 May 11, 2024
75 of 88 checks passed
jayralencar pushed a commit to jayralencar/autogen that referenced this pull request May 28, 2024
…ecutor (microsoft#2615)

* Add ability to use virtual environments in local executor

* Copy environment variables from parent environment

* Fix mypy errors and formatting

* Account for venv on Windows

* Use a virtual environment context object instead of path

* Add utility method to create a virtual environment

* Remove assertion using `_venv_path`

* Add tests for `create_virtual_env`

* Modify test code and add output assertion

* Modify test code and assertion

* Execute activation script before actual command on windows

* Add docs for using a virtual env
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.

None yet

3 participants