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

Fix: Unused HOST and PORT environment variables #1365

Merged

Conversation

imapersonman
Copy link
Contributor

Problem

My computer couldn't connect to the OI server running inside a docker container. This bug comes from this line, originating in commit 24e07c90845ebd1e18c11d4bfeea28f2d201c04f, and has persisted until this PR.

Before the commit linked above, it was possible to change the hostname of the server simply by setting the environment variable HOST. When running the OI server from inside a docker container, this env var needs to be set to 0.0.0.0 in order for the computer running the docker container to connect to the server. The commit did not remove the code for fetching the HOST and PORT env variables, but did stop referring to these variables in favor of hardcoded defaults.

Solution

I moved the code for fetching the HOST and PORT env vars into the Server's constructor.

Directions for testing

  1. poetry install -E server (If you don't have the server dependencies already installed)
  2. poetry run pytest tests/core/test_async_core.py

Pre-Submission Checklist (optional but appreciated):

  • I have included relevant documentation updates (stored in /docs)
  • I have read docs/CONTRIBUTING.md
  • I have read docs/ROADMAP.md

OS Tests (optional but appreciated):

  • Tested on Windows
  • Tested on MacOS
  • Tested on Linux

@KillianLucas KillianLucas changed the base branch from main to development July 26, 2024 23:28
@KillianLucas KillianLucas merged commit 54e2704 into OpenInterpreter:development Jul 26, 2024
0 of 2 checks passed
@KillianLucas
Copy link
Collaborator

Oh my god of course.. great catch Koissi, thanks for the fix! Makes the docker image work again for everyone. Merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants