From 593c4facb84b0a14dc150fbd70d33fd8237b06f8 Mon Sep 17 00:00:00 2001 From: Travis Dent Date: Mon, 23 Dec 2024 11:43:38 -0800 Subject: [PATCH 1/3] Environment variables are written commented-out if no value is set. This prevents us from overriding existing values in the user's env with placeholder values. --- agentstack/generation/files.py | 47 ++++++++++++++++++++++++---------- tests/fixtures/.env | 4 ++- tests/test_generation_files.py | 23 +++++++++++++++-- 3 files changed, 58 insertions(+), 16 deletions(-) diff --git a/agentstack/generation/files.py b/agentstack/generation/files.py index f2ad90a0..f6fa12c6 100644 --- a/agentstack/generation/files.py +++ b/agentstack/generation/files.py @@ -1,5 +1,6 @@ from typing import Optional, Union import os, sys +import string from pathlib import Path if sys.version_info >= (3, 11): @@ -9,7 +10,7 @@ from agentstack import conf -ENV_FILEMANE = ".env" +ENV_FILENAME = ".env" PYPROJECT_FILENAME = "pyproject.toml" @@ -17,10 +18,15 @@ class EnvFile: """ Interface for interacting with the .env file inside a project directory. Unlike the ConfigFile, we do not re-write the entire file on every change, - and instead just append new lines to the end of the file. This preseres + and instead just append new lines to the end of the file. This preserves comments and other formatting that the user may have added and prevents opportunities for data loss. + If the value of a variable is None, it will be commented out when it is written + to the file. This gives the user a suggestion, but doesn't override values that + may have been set by the user via other means (for example, but the user's shell). + Commented variable are not re-parsed when the file is read. + `path` is the directory where the .env file is located. Defaults to the current working directory. `filename` is the name of the .env file, defaults to '.env'. @@ -34,14 +40,14 @@ class EnvFile: variables: dict[str, str] - def __init__(self, filename: str = ENV_FILEMANE): + def __init__(self, filename: str = ENV_FILENAME): self._filename = filename self.read() - def __getitem__(self, key): + def __getitem__(self, key) -> str: return self.variables[key] - def __setitem__(self, key, value): + def __setitem__(self, key, value) -> None: if key in self.variables: raise ValueError("EnvFile does not allow overwriting values.") self.append_if_new(key, value) @@ -49,32 +55,47 @@ def __setitem__(self, key, value): def __contains__(self, key) -> bool: return key in self.variables - def append_if_new(self, key, value): + def append_if_new(self, key, value) -> None: + """Setting a non-existent key will append it to the end of the file.""" if key not in self.variables: self.variables[key] = value self._new_variables[key] = value - def read(self): - def parse_line(line): + def read(self) -> None: + def parse_line(line) -> tuple[str, Union[str, None]]: + """ + Parse a line from the .env file. + Pairs are split on the first '=' character, and stripped of whitespace & quotes. + If the value is empty, it is returned as None. + Only the last occurrence of a variable is stored. + """ key, value = line.split('=') - return key.strip(), value.strip() + key = key.strip() + value = value.strip(string.whitespace + '"') + return key, value if value else None if os.path.exists(conf.PATH / self._filename): with open(conf.PATH / self._filename, 'r') as f: - self.variables = dict([parse_line(line) for line in f.readlines() if '=' in line]) + self.variables = dict( + [parse_line(line) for line in f.readlines() if '=' in line and not line.startswith('#')] + ) else: self.variables = {} self._new_variables = {} - def write(self): + def write(self) -> None: + """Append new variables to the end of the file.""" with open(conf.PATH / self._filename, 'a') as f: for key, value in self._new_variables.items(): - f.write(f"\n{key}={value}") + if value is None: + f.write(f'\n#{key}=""') # comment-out empty values + else: + f.write(f'\n{key}={value}') def __enter__(self) -> 'EnvFile': return self - def __exit__(self, *args): + def __exit__(self, *args) -> None: self.write() diff --git a/tests/fixtures/.env b/tests/fixtures/.env index 3f1c8b1d..9197de0d 100644 --- a/tests/fixtures/.env +++ b/tests/fixtures/.env @@ -1,3 +1,5 @@ ENV_VAR1=value1 -ENV_VAR2=value2 \ No newline at end of file +ENV_VAR2=value_ignored +ENV_VAR2=value2 +#ENV_VAR3="" \ No newline at end of file diff --git a/tests/test_generation_files.py b/tests/test_generation_files.py index 900efdfd..91ad538b 100644 --- a/tests/test_generation_files.py +++ b/tests/test_generation_files.py @@ -93,7 +93,7 @@ def test_read_env(self): assert env["ENV_VAR1"] == "value1" assert env["ENV_VAR2"] == "value2" with self.assertRaises(KeyError) as _: - env["ENV_VAR3"] + env["ENV_VAR100"] def test_write_env(self): shutil.copy(BASE_PATH / "fixtures/.env", self.project_dir / ".env") @@ -103,4 +103,23 @@ def test_write_env(self): env.append_if_new("ENV_VAR100", "value2") # Should be added tmp_data = open(self.project_dir / ".env").read() - assert tmp_data == """\nENV_VAR1=value1\nENV_VAR2=value2\nENV_VAR100=value2""" + assert ( + tmp_data + == """\nENV_VAR1=value1\nENV_VAR2=value_ignored\nENV_VAR2=value2\n#ENV_VAR3=""\nENV_VAR100=value2""" + ) + + def test_write_env_commented(self): + """We should be able to write a commented-out value.""" + shutil.copy(BASE_PATH / "fixtures/.env", self.project_dir / ".env") + + with EnvFile() as env: + env.append_if_new("ENV_VAR3", "value3") + + env = EnvFile() # re-read the file + assert env.variables == {"ENV_VAR1": "value1", "ENV_VAR2": "value2", "ENV_VAR3": "value3"} + + tmp_file = open(self.project_dir / ".env").read() + assert ( + tmp_file + == """\nENV_VAR1=value1\nENV_VAR2=value_ignored\nENV_VAR2=value2\n#ENV_VAR3=""\nENV_VAR3=value3""" + ) From 8b7f43e5ec66b45bac0997a75fba66aa9f496242 Mon Sep 17 00:00:00 2001 From: Travis Dent Date: Mon, 23 Dec 2024 11:46:57 -0800 Subject: [PATCH 2/3] Update tool configs to use null values for placeholder environment variables. --- agentstack/tools/agent_connect.json | 8 ++++---- agentstack/tools/browserbase.json | 4 ++-- agentstack/tools/composio.json | 2 +- agentstack/tools/exa.json | 2 +- agentstack/tools/firecrawl.json | 2 +- agentstack/tools/ftp.json | 6 +++--- agentstack/tools/mem0.json | 2 +- agentstack/tools/neon.json | 2 +- agentstack/tools/perplexity.json | 2 +- agentstack/tools/stripe.json | 2 +- 10 files changed, 16 insertions(+), 16 deletions(-) diff --git a/agentstack/tools/agent_connect.json b/agentstack/tools/agent_connect.json index 3dd6a034..e0f41702 100644 --- a/agentstack/tools/agent_connect.json +++ b/agentstack/tools/agent_connect.json @@ -4,12 +4,12 @@ "category": "network-protocols", "packages": ["agent-connect"], "env": { - "HOST_DOMAIN": "...", + "HOST_DOMAIN": null, "HOST_PORT": 80, "HOST_WS_PATH": "/ws", - "DID_DOCUMENT_PATH": "...", - "SSL_CERT_PATH": "...", - "SSL_KEY_PATH": "..." + "DID_DOCUMENT_PATH": null, + "SSL_CERT_PATH": null, + "SSL_KEY_PATH": null }, "tools": ["send_message", "receive_message"] } diff --git a/agentstack/tools/browserbase.json b/agentstack/tools/browserbase.json index d005c278..6e442262 100644 --- a/agentstack/tools/browserbase.json +++ b/agentstack/tools/browserbase.json @@ -4,8 +4,8 @@ "category": "browsing", "packages": ["browserbase", "playwright"], "env": { - "BROWSERBASE_API_KEY": "...", - "BROWSERBASE_PROJECT_ID": "..." + "BROWSERBASE_API_KEY": null, + "BROWSERBASE_PROJECT_ID": null }, "tools": ["browserbase"], "cta": "Create an API key at https://www.browserbase.com/" diff --git a/agentstack/tools/composio.json b/agentstack/tools/composio.json index c2b20d0d..8af447ee 100644 --- a/agentstack/tools/composio.json +++ b/agentstack/tools/composio.json @@ -4,7 +4,7 @@ "category": "unified-apis", "packages": ["composio-crewai"], "env": { - "COMPOSIO_API_KEY": "..." + "COMPOSIO_API_KEY": null }, "tools": ["composio_tools"], "tools_bundled": true, diff --git a/agentstack/tools/exa.json b/agentstack/tools/exa.json index 2dada5e3..d8bc4679 100644 --- a/agentstack/tools/exa.json +++ b/agentstack/tools/exa.json @@ -4,7 +4,7 @@ "category": "web-retrieval", "packages": ["exa_py"], "env": { - "EXA_API_KEY": "..." + "EXA_API_KEY": null }, "tools": ["search_and_contents"], "cta": "Get your Exa API key at https://dashboard.exa.ai/api-keys" diff --git a/agentstack/tools/firecrawl.json b/agentstack/tools/firecrawl.json index 7937fde4..e85bca63 100644 --- a/agentstack/tools/firecrawl.json +++ b/agentstack/tools/firecrawl.json @@ -4,7 +4,7 @@ "category": "browsing", "packages": ["firecrawl-py"], "env": { - "FIRECRAWL_API_KEY": "..." + "FIRECRAWL_API_KEY": null }, "tools": ["web_scrape", "web_crawl", "retrieve_web_crawl"], "cta": "Create an API key at https://www.firecrawl.dev/" diff --git a/agentstack/tools/ftp.json b/agentstack/tools/ftp.json index ca11fcc8..81a41fe8 100644 --- a/agentstack/tools/ftp.json +++ b/agentstack/tools/ftp.json @@ -3,9 +3,9 @@ "category": "computer-control", "packages": [], "env": { - "FTP_HOST": "...", - "FTP_USER": "...", - "FTP_PASSWORD": "..." + "FTP_HOST": null, + "FTP_USER": null, + "FTP_PASSWORD": null }, "tools": ["upload_files"], "cta": "Be sure to add your FTP credentials to .env" diff --git a/agentstack/tools/mem0.json b/agentstack/tools/mem0.json index dfd224ac..919121b5 100644 --- a/agentstack/tools/mem0.json +++ b/agentstack/tools/mem0.json @@ -4,7 +4,7 @@ "category": "storage", "packages": ["mem0ai"], "env": { - "MEM0_API_KEY": "..." + "MEM0_API_KEY": null }, "tools": ["write_to_memory", "read_from_memory"], "cta": "Create your mem0 API key at https://mem0.ai/" diff --git a/agentstack/tools/neon.json b/agentstack/tools/neon.json index 8fd13f6d..aa3ebc91 100644 --- a/agentstack/tools/neon.json +++ b/agentstack/tools/neon.json @@ -4,7 +4,7 @@ "url": "https://github.com/neondatabase/neon", "packages": ["neon-api", "psycopg2-binary"], "env": { - "NEON_API_KEY": "..." + "NEON_API_KEY": null }, "tools": ["create_database", "execute_sql_ddl", "run_sql_query"], "cta": "Create an API key at https://www.neon.tech" diff --git a/agentstack/tools/perplexity.json b/agentstack/tools/perplexity.json index ba6fe696..90630723 100644 --- a/agentstack/tools/perplexity.json +++ b/agentstack/tools/perplexity.json @@ -3,7 +3,7 @@ "url": "https://perplexity.ai", "category": "search", "env": { - "PERPLEXITY_API_KEY": "..." + "PERPLEXITY_API_KEY": null }, "tools": ["query_perplexity"] } \ No newline at end of file diff --git a/agentstack/tools/stripe.json b/agentstack/tools/stripe.json index 212c6b23..91a73aae 100644 --- a/agentstack/tools/stripe.json +++ b/agentstack/tools/stripe.json @@ -4,7 +4,7 @@ "category": "application-specific", "packages": ["stripe-agent-toolkit", "stripe"], "env": { - "STRIPE_SECRET_KEY": "sk-..." + "STRIPE_SECRET_KEY": null }, "tools_bundled": true, "tools": ["stripe_tools"], From b75b2835c2871989164d406072da02accab00e2e Mon Sep 17 00:00:00 2001 From: Travis Dent Date: Mon, 23 Dec 2024 11:52:50 -0800 Subject: [PATCH 3/3] Don't override `false`` values as None when re-parsing env vars --- agentstack/generation/files.py | 7 ++----- tests/test_generation_files.py | 10 ++++++++++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/agentstack/generation/files.py b/agentstack/generation/files.py index f6fa12c6..678a16f5 100644 --- a/agentstack/generation/files.py +++ b/agentstack/generation/files.py @@ -62,17 +62,14 @@ def append_if_new(self, key, value) -> None: self._new_variables[key] = value def read(self) -> None: - def parse_line(line) -> tuple[str, Union[str, None]]: + def parse_line(line) -> tuple[str, str]: """ Parse a line from the .env file. Pairs are split on the first '=' character, and stripped of whitespace & quotes. - If the value is empty, it is returned as None. Only the last occurrence of a variable is stored. """ key, value = line.split('=') - key = key.strip() - value = value.strip(string.whitespace + '"') - return key, value if value else None + return key.strip(), value.strip(string.whitespace + '"') if os.path.exists(conf.PATH / self._filename): with open(conf.PATH / self._filename, 'r') as f: diff --git a/tests/test_generation_files.py b/tests/test_generation_files.py index 91ad538b..c650f0ac 100644 --- a/tests/test_generation_files.py +++ b/tests/test_generation_files.py @@ -107,6 +107,16 @@ def test_write_env(self): tmp_data == """\nENV_VAR1=value1\nENV_VAR2=value_ignored\nENV_VAR2=value2\n#ENV_VAR3=""\nENV_VAR100=value2""" ) + + def test_write_env_numeric_that_can_be_boolean(self): + shutil.copy(BASE_PATH / "fixtures/.env", self.project_dir / ".env") + + with EnvFile() as env: + env.append_if_new("ENV_VAR100", 0) + env.append_if_new("ENV_VAR101", 1) + + env = EnvFile() # re-read the file + assert env.variables == {"ENV_VAR1": "value1", "ENV_VAR2": "value2", "ENV_VAR100": "0", "ENV_VAR101": "1"} def test_write_env_commented(self): """We should be able to write a commented-out value."""