Skip to content

feat: add environment variable loading in S3Client initialization #45

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

Merged
amindadgar merged 1 commit into from
May 21, 2025
Merged

feat: add environment variable loading in S3Client initialization #45

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions hivemind_etl/storage/s3_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,15 @@
from botocore.config import Config
from botocore.exceptions import ClientError
from llama_index.core import Document
from dotenv import load_dotenv


class S3Client:
def __init__(self):
loaded = load_dotenv()
if not loaded:
raise ValueError("Failed to load environment variables")

Comment on lines +16 to +19
Copy link

@coderabbitai coderabbitai bot May 21, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Consider impact on testing environments.

This change could affect unit tests that rely on mocked environment variables. Ensure that your testing setup properly handles the .env file loading, either by providing a test .env file or by mocking the load_dotenv function.

🏁 Script executed:

#!/bin/bash
# Check for test files that might be affected
echo "Checking for tests that use S3Client:"
fd -e py "test.*\.py" | xargs grep -l "S3Client"

echo "Checking for test environment setup:"
fd -e py "test.*\.py" | xargs grep -l "dotenv\|getenv\|environ"

Length of output: 334

Ensure tests handle the new .env loading check

The added guard in hivemind_etl/storage/s3_client.py will raise a ValueError if load_dotenv() returns False, which can break existing unit tests that set environment variables via mocks rather than an actual .env file. Please update your testing setup to account for this:

• Tests referencing environment setup (may be affected):

  • test_run_workflow.py
  • tests/unit/test_website_etl.py

• Consider one of the following fixes:

  • Mock load_dotenv() in these tests so it always returns True.
  • Provide a lightweight test .env fixture loaded before S3Client is instantiated.
  • Refactor S3Client to accept an injectable “dotenv loader” or to skip the check under a test flag.
🤖 Prompt for AI Agents 
In hivemind_etl/storage/s3_client.py around lines 16 to 19, the code now raises
a ValueError if load_dotenv() returns False, which can break unit tests that
mock environment variables without an actual .env file. To fix this, update the
affected tests (e.g., test_run_workflow.py and tests/unit/test_website_etl.py)
by either mocking load_dotenv() to always return True, providing a minimal test
.env file loaded before S3Client instantiation, or refactoring S3Client to
accept a configurable dotenv loader or skip the check during testing.

# Get AWS S3 environment variables
self.endpoint_url = os.getenv("AWS_ENDPOINT_URL")
self.access_key = os.getenv("AWS_ACCESS_KEY_ID")
Expand Down