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

set use_docker to default to True #1147

Merged
merged 25 commits into from
Jan 18, 2024
Merged

Conversation

olgavrou
Copy link
Member

@olgavrou olgavrou commented Jan 4, 2024

Why are these changes needed?

Related issue number

Resolves #1103

Comments:

  • If we are setting use_docker to default to True then docker should be a required dependency
  • It is more user friendly to check and warn for docker running/not running when the agent is setup rather than when it tries to execute, the message gets lost in the wall of text that is outputted

TODOs:

Checks

@codecov-commenter
Copy link

codecov-commenter commented Jan 4, 2024

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (22e36cb) 32.06% compared to head (f1455a6) 49.88%.

Files Patch % Lines
autogen/code_utils.py 75.00% 6 Missing and 4 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1147       +/-   ##
===========================================
+ Coverage   32.06%   49.88%   +17.82%     
===========================================
  Files          33       33               
  Lines        4422     4456       +34     
  Branches     1034     1096       +62     
===========================================
+ Hits         1418     2223      +805     
+ Misses       2887     2012      -875     
- Partials      117      221      +104     
Flag Coverage Δ
unittests 49.88% <78.26%> (+17.86%) ⬆️

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.

website/docs/Installation.md Outdated Show resolved Hide resolved
website/docs/Installation.md Outdated Show resolved Hide resolved
@sonichi sonichi added this pull request to the merge queue Jan 18, 2024
Merged via the queue into microsoft:main with commit a911d1c Jan 18, 2024
97 checks passed
corleroux pushed a commit to corleroux/autogen that referenced this pull request Jan 30, 2024
* set use_docker to default to true

* black formatting

* centralize checking and add env variable option

* set docker env flag for contrib tests

* set docker env flag for contrib tests

* better error message and cleanup

* disable explicit docker tests

* docker is installed so can't check for that in test

* pr comments and fix test

* rename and fix function descriptions

* documentation

* update notebooks so that they can be run with change in default

* add unit tests for new code

* cache and restore env var

* skip on windows because docker is running in the CI but there are problems connecting the volume

* update documentation

* move header

* update contrib tests
whiskyboy pushed a commit to whiskyboy/autogen that referenced this pull request Apr 17, 2024
* set use_docker to default to true

* black formatting

* centralize checking and add env variable option

* set docker env flag for contrib tests

* set docker env flag for contrib tests

* better error message and cleanup

* disable explicit docker tests

* docker is installed so can't check for that in test

* pr comments and fix test

* rename and fix function descriptions

* documentation

* update notebooks so that they can be run with change in default

* add unit tests for new code

* cache and restore env var

* skip on windows because docker is running in the CI but there are problems connecting the volume

* update documentation

* move header

* update contrib tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-execution execute generated code docker Issues relating to Docker
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request]: Set use_docker to True if unspecified
9 participants