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

[WIP]Feature, configurable network port #6678

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion autogpts/autogpt/autogpt/app/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ async def run_auto_gpt_server(
server = AgentProtocolServer(
app_config=config, database=database, llm_provider=llm_provider
)
await server.start()
await server.start(port=config.serving_port)
Copy link
Member

Choose a reason for hiding this comment

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

does not match the config attribute created in config.py



def _configure_openai_provider(config: Config) -> OpenAIProvider:
Expand Down
4 changes: 4 additions & 0 deletions autogpts/autogpt/autogpt/config/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ class Config(SystemSettings, arbitrary_types_allowed=True):
default=6379,
from_env=lambda: int(v) if (v := os.getenv("REDIS_PORT")) else None,
)
agent_port: int = UserConfigurable(
default=8000,
from_env=lambda: int(v) if (v := os.getenv("AGENT_PORT")) else None,
)
Comment on lines +118 to +121
Copy link
Member

Choose a reason for hiding this comment

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

This should be in the "Application Settings" section, e.g. on line 55. Also, can you rename agent_port to serve_port or agent_serve_port?

redis_password: str = UserConfigurable("", from_env="REDIS_PASSWORD")
wipe_redis_on_start: bool = UserConfigurable(
default=True,
Expand Down
2 changes: 1 addition & 1 deletion autogpts/autogpt/run
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/bin/sh

kill $(lsof -t -i :8000)
kill $(lsof -t -i :${AGENT_PORT:-8080})
Copy link
Member

Choose a reason for hiding this comment

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

  1. This should default to 8000 like it does now
  2. This will break the benchmark when AGENT_PORT is not set to its default, because AGBenchmark uses the host property (including port) from agbenchmark_config/config.json to connect to the subject agent's API.


if [ ! -f .env ] && [ -z "$OPENAI_API_KEY" ]; then
cp .env.example .env
Expand Down
4 changes: 2 additions & 2 deletions autogpts/autogpt/run_benchmark
Copy link
Member

Choose a reason for hiding this comment

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

AGBenchmark reads PORT, not BENCHMARK_PORT:

# Run the FastAPI application using uvicorn
port = port or int(os.getenv("PORT", 8080))
uvicorn.run(app, host="0.0.0.0", port=port)

Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
#!/bin/sh

# Kill processes using port 8080 if any.
if lsof -t -i :8080; then
kill $(lsof -t -i :8080)
if lsof -t -i :${BENCHMARK_PORT:-8000}; then
kill $(lsof -t -i :${BENCHMARK_PORT:-8000})
fi
# This is the cli entry point for the benchmarking tool.
# To run this in server mode pass in `serve` as the first argument.
Expand Down
7 changes: 5 additions & 2 deletions cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,11 @@ def stop():
import signal
import subprocess

port = os.getenv('PORT', 8000)
agent_port = os.getenv('AGENT_PORT', 8080)

try:
pids = subprocess.check_output(["lsof", "-t", "-i", ":8000"]).split()
pids = subprocess.check_output(["lsof", "-t", "-i", ":"+str(agent_port)]).split()
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pids = subprocess.check_output(["lsof", "-t", "-i", ":"+str(agent_port)]).split()
pids = subprocess.check_output(["lsof", "-t", "-i", f":{agent_port}"]).split()

if isinstance(pids, int):
os.kill(int(pids), signal.SIGTERM)
else:
Expand All @@ -314,7 +317,7 @@ def stop():
click.echo("No process is running on port 8000")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
click.echo("No process is running on port 8000")
click.echo(f"No process is running on port {agent_port}")


try:
pids = int(subprocess.check_output(["lsof", "-t", "-i", ":8080"]))
pids = int(subprocess.check_output(["lsof", "-t", "-i", ":"+str(port)]))
if isinstance(pids, int):
os.kill(int(pids), signal.SIGTERM)
else:
Expand Down
6 changes: 4 additions & 2 deletions run
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#!/bin/bash

python3 cli.py "$@"
set -a
source .env
Copy link
Member

Choose a reason for hiding this comment

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

For most users, .env does not exist and this shows a warning.

set +a
python3 cli.py "$@"
Copy link
Member

Choose a reason for hiding this comment

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

Missing \n