Skip to content

Commit 0a4fd9f

Browse files
kenjudyclaude
andcommitted
Fix: Update tests for optional CLI params and resolve CI issues
- Fix test_parse_arguments_missing_required_arg to test validate_required_args - Fix test_network_timeout to expect RateLimitError instead of TimeoutError - Add comprehensive test coverage for validate_required_args (7 new tests) - Test all args provided, missing api_key/project/limit - Test environment variable support and mixed CLI/env usage - Add #nosec B110 to suppress intentional try/except/pass - Update README: Add env var support to features, update test count to 25 - Fix Windows venv activation commands (remove 'source') All 25 tests passing, 86% coverage, all CI checks green. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 0ba0adb commit 0a4fd9f

File tree

3 files changed

+174
-34
lines changed

3 files changed

+174
-34
lines changed

README.md

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,13 @@ This Python script exports trace data from LangSmith using the SDK API, designed
99
## Features
1010

1111
- Export N most recent traces from any LangSmith project
12+
- **Environment variable support** - Configure once via `.env` file for simplified usage
1213
- Automatic rate limiting with exponential backoff
1314
- Progress indication for long-running exports
1415
- Comprehensive error handling (auth, network, rate limits)
1516
- Structured JSON output with metadata
1617
- Type-safe implementation with full type hints
17-
- Test-driven development with pytest suite
18+
- Test-driven development with pytest suite (25 tests, 86% coverage)
1819

1920
## Requirements
2021

@@ -36,15 +37,15 @@ cd export-langsmith-data
3637
**Option A: Using uv (recommended)**
3738
```bash
3839
uv venv
39-
source .venv/Scripts/activate # Windows
40+
.venv/Scripts/activate # Windows
4041
# or
4142
source .venv/bin/activate # Linux/Mac
4243
```
4344

4445
**Option B: Using venv**
4546
```bash
4647
python -m venv .venv
47-
source .venv/Scripts/activate # Windows
48+
.venv/Scripts/activate # Windows
4849
# or
4950
source .venv/bin/activate # Linux/Mac
5051
```
@@ -225,14 +226,15 @@ All core features implemented and tested:
225226
- ✅ Project setup with virtual environment (uv/venv)
226227
- ✅ Dependencies configuration with CI/CD quality gates
227228
- ✅ CLI argument parsing with validation
229+
-**Environment variable support** - Optional `.env` file configuration for simplified usage
228230
- ✅ LangSmith client initialization with authentication
229231
- ✅ Run fetching with exponential backoff rate limiting
230232
- ✅ Data formatting and transformation with safe field extraction
231233
- ✅ JSON export functionality with error handling
232234
- ✅ Comprehensive error scenario handling
233235
- ✅ Main orchestration with user-friendly progress feedback
234236
- ✅ End-to-end integration testing
235-
- ✅ Test suite: 18 tests, 87% coverage
237+
- ✅ Test suite: 25 tests, 86% coverage
236238
- ✅ Code quality: Black, Ruff, mypy, Bandit, Safety checks passing
237239

238240
### Optional Features Not Implemented

export_langsmith_traces.py

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,10 @@ def __init__(self, api_key: str, api_url: str = DEFAULT_API_URL) -> None:
9292
def _looks_like_uuid(self, value: str) -> bool:
9393
"""Check if a string looks like a UUID."""
9494
import re
95+
9596
uuid_pattern = re.compile(
96-
r'^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$',
97-
re.IGNORECASE
97+
r"^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$",
98+
re.IGNORECASE,
9899
)
99100
return bool(uuid_pattern.match(value))
100101

@@ -115,7 +116,7 @@ def fetch_runs(self, project_name: str, limit: int) -> List[Any]:
115116
"""
116117
attempt = 0
117118
last_exception = None
118-
119+
119120
while attempt < self.MAX_RETRIES:
120121
try:
121122
# Try with project_name parameter first
@@ -126,20 +127,25 @@ def fetch_runs(self, project_name: str, limit: int) -> List[Any]:
126127
except Exception as e:
127128
last_exception = e
128129
error_msg = str(e).lower()
129-
130+
130131
# Check if this is a project not found error (not a rate limit or network error)
131-
if any(term in error_msg for term in ["not found", "does not exist", "project", "404"]):
132+
if any(
133+
term in error_msg
134+
for term in ["not found", "does not exist", "project", "404"]
135+
):
132136
# If it looks like a UUID, try as project_id instead
133137
if self._looks_like_uuid(project_name):
134138
print("Trying project ID instead of name...")
135139
try:
136140
runs = list(
137-
self.client.list_runs(project_id=project_name, limit=limit)
141+
self.client.list_runs(
142+
project_id=project_name, limit=limit
143+
)
138144
)
139145
return runs
140-
except Exception:
141-
pass # Fall through to retry logic
142-
146+
except Exception: # nosec B110
147+
pass # Intentional: Fall through to retry logic if project_id also fails
148+
143149
# If first attempt and looks like project name issue, raise specific error
144150
if attempt == 0:
145151
raise ProjectNotFoundError(
@@ -148,7 +154,7 @@ def fetch_runs(self, project_name: str, limit: int) -> List[Any]:
148154
f"You can find the project ID in the LangSmith URL when viewing your project. "
149155
f"Original error: {str(e)}"
150156
) from e
151-
157+
152158
attempt += 1
153159
if attempt >= self.MAX_RETRIES:
154160
break
@@ -157,7 +163,7 @@ def fetch_runs(self, project_name: str, limit: int) -> List[Any]:
157163
self.BACKOFF_MULTIPLIER ** (attempt - 1)
158164
)
159165
time.sleep(backoff_time)
160-
166+
161167
# If we get here, all retries failed
162168
raise RateLimitError(
163169
f"Failed to fetch runs after {self.MAX_RETRIES} attempts. "
@@ -274,10 +280,10 @@ def _get_env_limit() -> int:
274280
limit_str = os.getenv("LANGSMITH_LIMIT", "0")
275281
limit = int(limit_str)
276282
if limit <= 0:
277-
return 0
283+
return 0
278284
return limit
279285
except ValueError:
280-
return 0
286+
return 0
281287

282288

283289
def parse_arguments() -> argparse.Namespace:
@@ -289,7 +295,7 @@ def parse_arguments() -> argparse.Namespace:
289295
"""
290296
# Load environment variables from .env file
291297
load_dotenv()
292-
298+
293299
parser = argparse.ArgumentParser(
294300
description="Export LangSmith trace data for offline analysis",
295301
formatter_class=argparse.RawDescriptionHelpFormatter,
@@ -325,7 +331,7 @@ def parse_arguments() -> argparse.Namespace:
325331
type=str,
326332
required=False,
327333
default=os.getenv("LANGSMITH_PROJECT"),
328-
help="LangSmith project name or ID (default: LANGSMITH_PROJECT env var)"
334+
help="LangSmith project name or ID (default: LANGSMITH_PROJECT env var)",
329335
)
330336

331337
parser.add_argument(
@@ -346,24 +352,24 @@ def parse_arguments() -> argparse.Namespace:
346352
def validate_required_args(args: argparse.Namespace) -> None:
347353
"""
348354
Validate that required arguments are provided via CLI or environment.
349-
355+
350356
Args:
351357
args: Parsed command line arguments
352-
358+
353359
Raises:
354360
SystemExit: If required arguments are missing
355361
"""
356362
errors = []
357-
363+
358364
if not args.api_key:
359365
errors.append("--api-key is required (or set LANGSMITH_API_KEY in .env)")
360-
366+
361367
if not args.project:
362368
errors.append("--project is required (or set LANGSMITH_PROJECT in .env)")
363-
369+
364370
if not args.limit:
365371
errors.append("--limit is required (or set LANGSMITH_LIMIT in .env)")
366-
372+
367373
if errors:
368374
print("❌ Missing required arguments:", file=sys.stderr)
369375
for error in errors:
@@ -399,7 +405,7 @@ def main() -> None:
399405
try:
400406
# Parse arguments
401407
args = parse_arguments()
402-
408+
403409
# Validate that required arguments are available
404410
validate_required_args(args)
405411

test_export_langsmith_traces.py

Lines changed: 140 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,10 @@
1515
from export_langsmith_traces import (
1616
LangSmithExporter,
1717
parse_arguments,
18+
validate_required_args,
1819
AuthenticationError,
1920
ExportError,
21+
RateLimitError,
2022
)
2123

2224

@@ -48,8 +50,8 @@ def test_parse_arguments_with_required_args(self):
4850
assert args.output == "test_output.json"
4951

5052
def test_parse_arguments_missing_required_arg(self):
51-
"""Test that missing required argument raises error."""
52-
# Arrange - missing --api-key argument
53+
"""Test that missing required argument (no env var fallback) raises error."""
54+
# Arrange - missing --api-key argument and no env var
5355
test_args = [
5456
"--project",
5557
"test-project",
@@ -59,10 +61,20 @@ def test_parse_arguments_missing_required_arg(self):
5961
"test_output.json",
6062
]
6163

62-
# Act & Assert - should raise SystemExit
63-
with patch("sys.argv", ["export_langsmith_traces.py"] + test_args):
64-
with pytest.raises(SystemExit):
65-
parse_arguments()
64+
# Act & Assert - parse_arguments with no env vars, then validate should fail
65+
with patch.dict(
66+
"os.environ", {}, clear=True
67+
): # Clear all env vars before parsing
68+
with patch(
69+
"export_langsmith_traces.load_dotenv"
70+
): # Prevent loading .env file
71+
with patch("sys.argv", ["export_langsmith_traces.py"] + test_args):
72+
args = parse_arguments()
73+
# api_key should be None since not provided via CLI or env
74+
assert args.api_key is None
75+
# Now validate_required_args should raise SystemExit
76+
with pytest.raises(SystemExit):
77+
validate_required_args(args)
6678

6779
def test_parse_arguments_invalid_limit(self):
6880
"""Test that negative or zero limit is rejected."""
@@ -84,6 +96,123 @@ def test_parse_arguments_invalid_limit(self):
8496
parse_arguments()
8597

8698

99+
class TestValidateRequiredArgs:
100+
"""Test validate_required_args function with various argument combinations."""
101+
102+
def test_validate_with_all_args_provided(self):
103+
"""Test validation passes when all required args are provided."""
104+
# Arrange
105+
from argparse import Namespace
106+
107+
args = Namespace(
108+
api_key="test_key", project="test-project", limit=100, output="test.json"
109+
)
110+
111+
# Act & Assert - should not raise
112+
validate_required_args(args)
113+
114+
def test_validate_missing_api_key(self):
115+
"""Test validation fails when api_key is missing."""
116+
# Arrange
117+
from argparse import Namespace
118+
119+
args = Namespace(
120+
api_key=None, project="test-project", limit=100, output="test.json"
121+
)
122+
123+
# Act & Assert
124+
with pytest.raises(SystemExit):
125+
validate_required_args(args)
126+
127+
def test_validate_missing_project(self):
128+
"""Test validation fails when project is missing."""
129+
# Arrange
130+
from argparse import Namespace
131+
132+
args = Namespace(
133+
api_key="test_key", project=None, limit=100, output="test.json"
134+
)
135+
136+
# Act & Assert
137+
with pytest.raises(SystemExit):
138+
validate_required_args(args)
139+
140+
def test_validate_missing_limit(self):
141+
"""Test validation fails when limit is missing."""
142+
# Arrange
143+
from argparse import Namespace
144+
145+
args = Namespace(
146+
api_key="test_key", project="test-project", limit=None, output="test.json"
147+
)
148+
149+
# Act & Assert
150+
with pytest.raises(SystemExit):
151+
validate_required_args(args)
152+
153+
def test_validate_multiple_missing_args(self):
154+
"""Test validation fails when multiple args are missing."""
155+
# Arrange
156+
from argparse import Namespace
157+
158+
args = Namespace(api_key=None, project=None, limit=None, output="test.json")
159+
160+
# Act & Assert
161+
with pytest.raises(SystemExit):
162+
validate_required_args(args)
163+
164+
def test_parse_with_env_vars(self):
165+
"""Test that args can be provided via environment variables."""
166+
# Arrange
167+
test_args = ["--output", "test.json"]
168+
169+
# Act
170+
with patch.dict(
171+
"os.environ",
172+
{
173+
"LANGSMITH_API_KEY": "env_api_key",
174+
"LANGSMITH_PROJECT": "env_project",
175+
"LANGSMITH_LIMIT": "200",
176+
},
177+
):
178+
with patch("sys.argv", ["export_langsmith_traces.py"] + test_args):
179+
args = parse_arguments()
180+
181+
# Assert
182+
assert args.api_key == "env_api_key"
183+
assert args.project == "env_project"
184+
assert args.limit == 200
185+
assert args.output == "test.json"
186+
187+
# Validate should pass
188+
validate_required_args(args)
189+
190+
def test_parse_with_mixed_cli_and_env(self):
191+
"""Test that CLI args override environment variables."""
192+
# Arrange
193+
test_args = ["--api-key", "cli_key", "--output", "test.json"]
194+
195+
# Act
196+
with patch.dict(
197+
"os.environ",
198+
{
199+
"LANGSMITH_API_KEY": "env_key",
200+
"LANGSMITH_PROJECT": "env_project",
201+
"LANGSMITH_LIMIT": "200",
202+
},
203+
):
204+
with patch("sys.argv", ["export_langsmith_traces.py"] + test_args):
205+
args = parse_arguments()
206+
207+
# Assert - CLI should override env
208+
assert args.api_key == "cli_key" # From CLI
209+
assert args.project == "env_project" # From env
210+
assert args.limit == 200 # From env
211+
212+
# Validate should pass
213+
validate_required_args(args)
214+
215+
87216
class TestLangSmithExporter:
88217
"""Test LangSmithExporter class methods."""
89218

@@ -439,10 +568,13 @@ def test_network_timeout(self, mock_sleep, mock_client_class):
439568

440569
exporter = LangSmithExporter(api_key="test_key")
441570

442-
# Act & Assert - should retry and eventually fail
443-
with pytest.raises(TimeoutError):
571+
# Act & Assert - should retry and eventually raise RateLimitError
572+
with pytest.raises(RateLimitError) as exc_info:
444573
exporter.fetch_runs(project_name="test-project", limit=10)
445574

575+
# Verify the error message contains info about the timeout
576+
assert "Connection timeout" in str(exc_info.value)
577+
446578
# Should have retried MAX_RETRIES times
447579
assert mock_client.list_runs.call_count == exporter.MAX_RETRIES
448580

0 commit comments

Comments
 (0)