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.

Sign up for GitHub

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

Jump to bottom

Container improvements #540

Merged
merged 18 commits into from
Aug 10, 2024
Merged

Container improvements #540

merged 18 commits into from
Aug 10, 2024

Conversation

Robinsane
Copy link
Contributor

  1. added 3 env variables to make life easier configuring shell_gpt containers:
    IN_CONTAINER: setting this one to true, helps fix some default things that aren't handy when in a container:
  • The default for interaction becomes False, since [E]xecute on host doesn't work when you get the output from a container
  • Shell_gpt will not check the machine for os and shell (which would be fetched from the container), but will check what the user has configured in following env vars: OS_OUTSIDE_CONTAINER and SHELL_OUTSIDE_CONTAINER
  1. adjusted Dockerfile to use new env vars + added an example ollama Dockerfile

This was referenced Apr 11, 2024
Open
Open
@Robinsane Robinsane force-pushed the container_improvements branch from c93b6d7 to 98e2c32 Compare April 11, 2024 15:41
TheR1D
TheR1D requested changes Apr 12, 2024
View reviewed changes
Copy link
Owner

@TheR1D TheR1D left a comment

Choose a reason for hiding this comment

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

Could we also update Config and Docker section in README.md please?

sgpt/config.py Outdated Show resolved Hide resolved
sgpt/app.py Outdated Show resolved Hide resolved
Dockerfile_ollama Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
sgpt/role.py Outdated Show resolved Hide resolved
@ismaelchris
Copy link

These settings worked for me:

IN_CONTAINER=false
USE_LITELLM=true

@Robinsane Robinsane requested a review from TheR1D April 15, 2024 13:56
TheR1D
TheR1D requested changes Apr 23, 2024
View reviewed changes
sgpt/app.py Outdated Show resolved Hide resolved
sgpt/role.py Outdated Show resolved Hide resolved
sgpt/role.py Outdated Show resolved Hide resolved
sgpt/config.py Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Robinsane and others added 5 commits April 24, 2024 09:57
@Robinsane
edited example to output pretty OS name
97b2a3a
@Robinsane @TheR1D
OVERWRITE_OS_NAME
e04f2fe 
default OVERWRITE_OS_NAME value "default" + got rid of redundant else

Co-authored-by: Farkhod Sadykov <Farkhod.Sadykov@gmail.com>
@Robinsane @TheR1D
OVERWRITE_SHELL_NAME
f874b5f 
OVERWRITE_SHELL_NAME default value "default" + got rid of redundant else

Co-authored-by: Farkhod Sadykov <Farkhod.Sadykov@gmail.com>
@Robinsane @TheR1D
default values for OVERWRITE_SHELL_NAME and OVERWRITE_SHELL_NAME
1143d73 
Co-authored-by: Farkhod Sadykov <Farkhod.Sadykov@gmail.com>
@Robinsane
default overwrite values in Dockerfile
b510b72
@Robinsane Robinsane requested a review from TheR1D April 24, 2024 08:36
@Robinsane
Copy link
Contributor Author

Please give some feedback / merge when you have spare time :)

@TheR1D TheR1D added the docker Docker related label Jun 20, 2024
Robinsane added 4 commits June 20, 2024 10:10
@Robinsane
Less confusing variables: OS_NAME and SHELL_NAME with default value auto
9ef27fb
@Robinsane
Change as desired by TheR1D, in order to keep the code base consistent.
a12d846 
(Note: if SHELL_INTERACTION contains an unintended / weird value, it will act as if it contained false, which is not the intended default behaviour)
@Robinsane
doc changes: renamed env vars + OS_NAME =$(uname -s) in order for it …
fdff2c0 
…to work on Mac too
@Robinsane
rebase on main
e1d8f49
@Robinsane Robinsane force-pushed the container_improvements branch from 07c43e1 to 1c7ac42 Compare June 20, 2024 09:42
@Robinsane Robinsane force-pushed the container_improvements branch from 9163dfe to 00da9de Compare June 20, 2024 10:50
@Robinsane
Copy link
Contributor Author

Changes made + tested in our production environment.
I believe this is ready to be merged :)

TheR1D
TheR1D requested changes Jun 21, 2024
View reviewed changes
Copy link
Owner

@TheR1D TheR1D left a comment

Choose a reason for hiding this comment

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

Thank you for the updates @Robinsane. I ran some tests as well and have a few suggestions.

README.md Outdated Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@Robinsane @TheR1D
Update README.md
cfaa71b 
suggestion TheR1D

Co-authored-by: Farkhod Sadykov <Farkhod.Sadykov@gmail.com>
Robinsane and others added 2 commits June 21, 2024 09:13
@Robinsane @TheR1D
Dockerfile env var
d3b36d6 
ENV PRETTIFY_MARKDOWN=false

Co-authored-by: Farkhod Sadykov <Farkhod.Sadykov@gmail.com>
@Robinsane
Clear explanation of what to do when you want to use Ollama for LLM r…
6d213fd 
…equests in combination with ShellGPT in a docker container
@Robinsane Robinsane requested a review from TheR1D June 21, 2024 07:41
@Robinsane Robinsane mentioned this pull request Jun 21, 2024
Open
@TheR1D TheR1D merged commit f0d000a into TheR1D:main Aug 10, 2024
@TheR1D
Copy link
Owner

TheR1D commented Aug 10, 2024

Hi @Robinsane, Sorry for the late reply. The PR has been merged. Thank you for your contribution!

giladbarnea pushed a commit to giladbarnea/shell_gpt that referenced this pull request Nov 2, 2024
@tijszwinkels @giladbarnea
Adopt new function calling that supports Groq and other models (TheR1…
1970981 
…D#569)

Co-authored-by: Hejia Zhang <hejiazhang2023@outlook.com>

Docker container improvements (TheR1D#540)

Co-authored-by: Farkhod Sadykov <Farkhod.Sadykov@gmail.com>

Minor bug fixes, documentation, release 1.4.4 (TheR1D#607)

CD pipeline fix (TheR1D#608)

Option for LiteLLM to manage API keys (TheR1D#604)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TheR1D TheR1D TheR1D approved these changes

Assignees
No one assigned
Labels
docker Docker related
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Robinsane @ismaelchris @TheR1D