refactor: decouple UpstashDriver from env loading (Item 15)

autograder/services/upstash_driver.py

@@ -1,23 +1,22 @@
 import json
 import logging
-import os
 from typing import Optional
-from dotenv import load_dotenv
 from upstash_redis import Redis
 
 from autograder.models.abstract.exporter import Exporter
 
 logger = logging.getLogger(__name__)
 
-load_dotenv() #TODO: place this in application startup
+
 class UpstashDriver(Exporter):
-    def __init__(self):
-        self.redis = Redis(
-            os.getenv("UPSTASH_REDIS_URL"),
-            os.getenv("UPSTASH_REDIS_TOKEN") # should it do it? should it remain expecting the Redis python object?
-        )
+    def __init__(self, redis_url: str, redis_token: str):
+        """Initialize the driver with explicit credentials."""
+        if not redis_url or not redis_token:
+            raise ValueError("UpstashDriver requires both redis_url and redis_token.")
+
+        self.redis = Redis(redis_url, redis_token)
 
-    def get_user_quota(self,user_credentials: str) -> int:
+    def get_user_quota(self, user_credentials: str) -> int:
         """Function to get the quota of a user based on his username"""
         key = f"user:{user_credentials}"
         result = self.redis.hget(key, "quota")

autograder/steps/step_registry.py

@@ -1,3 +1,4 @@
+import os
 from typing import Dict, Callable, Any, Optional
 
 from autograder.models.dataclass.step_result import StepName
@@ -79,7 +80,11 @@ def _build_feedback(self) -> Optional[Step]:
 
     def _build_exporter(self) -> Optional[Step]:
         if self.config.get("export_results"):
-            exporter = self.config.get("exporter") or UpstashDriver()
+            # Update the fallback to include the required credentials
+            exporter = self.config.get("exporter") or UpstashDriver(
+                redis_url=os.getenv("UPSTASH_REDIS_URL"),
+                redis_token=os.getenv("UPSTASH_REDIS_TOKEN")
+            )
             return ExporterStep(exporter)
         return None
 

github_action/main.py

@@ -7,6 +7,7 @@
 
 import logging
 import os
+from dotenv import load_dotenv
 from argparse import ArgumentParser
 from .github_action_service import GithubActionService
 from autograder.autograder import AutograderPipeline
@@ -59,6 +60,9 @@ async def main():
     This makes the Adapter accessible to the GitHub Action workflow,
     that runs by entrypoint.sh script with all arguments passed to it.
     """
+
+    load_dotenv()  # Load environment variables from .env file if present
+
     success_execution = False
     try:
         args = __parser_values()

tests/unit/pipeline/test_exporter_interface.py

@@ -1,5 +1,5 @@
 import pytest
-from unittest.mock import MagicMock, patch
+from unittest.mock import MagicMock, patch, ANY
 from typing import Optional
 
 from autograder.models.abstract.exporter import Exporter
@@ -43,7 +43,8 @@ def test_exporter_step_calls_export(self):
     def test_upstash_driver_export_delegates(self):
         """UpstashDriver.export() delegates to set_score()."""
         with patch("autograder.services.upstash_driver.Redis"):
-            driver = UpstashDriver()
+            # Pass dummy credentials to satisfy the new __init__ parameters
+            driver = UpstashDriver(redis_url="mock_url", redis_token="mock_token")
             driver.set_score = MagicMock()
 
             driver.export("user123", 95.0, "Optional feedback")
@@ -85,4 +86,11 @@ def test_step_registry_build_exporter_default(self):
 
             assert isinstance(step, ExporterStep)
             assert step._exporter_service is mock_driver
-            mock_driver_cls.assert_called_once()
+
+            # Verify that the driver is initialized with environment variables.
+            # UpstashDriver.__init__ handles validation and raises ValueError if 
+            # redis_url or redis_token are missing (None).
+            mock_driver_cls.assert_called_once_with(
+                redis_url=ANY, 
+                redis_token=ANY
+            )

web/core/lifespan.py

@@ -1,9 +1,11 @@
 """Application lifespan management."""
 
 import asyncio
+import os
 from contextlib import asynccontextmanager
 from typing import Optional
 
+from dotenv import load_dotenv
 from fastapi import FastAPI
 
 from autograder.services.template_library_service import TemplateLibraryService
@@ -44,6 +46,9 @@ async def lifespan(app: FastAPI):
     Shutdown:
     - Clean up resources
     """
+
+    load_dotenv()  # Load environment variables first from .env file
+
     global template_service, grading_tasks
 
     # Startup