Refac NASA service and test impl by simlal · Pull Request #8 · simlal/aws-lambda-python-template

simlal · 2026-03-06T19:04:21Z

No description provided.

coderabbitai · 2026-03-06T19:04:35Z

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: fd193473-7a11-4b06-bad8-5987235d3216

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch gemini-improvements-test-ide-plugins-copilot

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Copilot

Pull request overview

This PR refactors the Lambda implementation into a “manager” orchestration flow, extracts NASA and DB-migrations logic into dedicated service/repository modules, and adds an initial unit test + testing docs.

Changes:

Refactor lambda_function.py into LambdaManager + LambdaResponseFormatter and extract NASA/migrations logic into services/.
Add custom exception types under models/exceptions.py.
Add basic unit test scaffolding and document test execution; introduce tooling/config files.

Reviewed changes

Copilot reviewed 9 out of 15 changed files in this pull request and generated 14 comments.

Show a summary per file

File	Description
`lambda_function.py`	Major refactor: manager-based orchestration, centralized success/error formatting, calls into new NASA/migrations components
`services/nasa.py`	New NASA APOD service wrapper around `ExternalApiService`
`services/migrations.py`	New repository for reading latest DB migrations from `SequelizeMeta`
`models/exceptions.py`	New custom exception types used by the refactored Lambda
`tests/test_lambda_function.py`	New unit tests (currently includes placeholder + event fixture issues)
`pyproject.toml`	Adds `pytest` dependency
`README.md`	Updates project structure docs and adds a testing section
`.pr_agent.toml`, `.coderabbit.yaml`	Adds config that disables automated review features
`.idea/*`	Adds IDE project files

Files not reviewed (5)

.idea/.gitignore: Language not supported
.idea/aws-lambda-python-template.iml: Language not supported
.idea/misc.xml: Language not supported
.idea/modules.xml: Language not supported
.idea/vcs.xml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-06T19:10:43Z

services/nasa.py

+from requests.exceptions import RequestException
+from requests.models import HTTPError
+
+from models.response import NasaApiResponse


NasaApiResponse is imported but never used in this module, which will fail Ruff's unused-import check (F401). Remove the import or use it for response validation here.

Suggested change

from models.response import NasaApiResponse

Copilot · 2026-03-06T19:10:44Z

.idea/vcs.xml

+  <component name="VcsDirectoryMappings">
+    <mapping directory="" vcs="Git" />
+    <mapping directory="$PROJECT_DIR$" vcs="Git" />
+  </component>


IDE-specific .idea/ project files are being committed, but .gitignore doesn't exclude them. This typically causes noisy diffs and environment-specific config to leak into the repo. Consider removing .idea/ from version control and adding .idea/ to .gitignore.

Suggested change

<component name="VcsDirectoryMappings">

<mapping directory="" vcs="Git" />

<mapping directory="$PROJECT_DIR$" vcs="Git" />

</component>

Copilot · 2026-03-06T19:10:44Z

.pr_agent.toml

+[config]
+disable_auto_feedback = true


This adds configuration that disables PR agent auto feedback. This is unrelated to the PR title and can change repo automation behavior; confirm it's intended (and document the rationale if so).

Copilot · 2026-03-06T19:10:44Z

.coderabbit.yaml

+# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
+reviews:
+  auto_review:
+    enabled: false


This disables CodeRabbit auto reviews. Since it's unrelated to the NASA refactor, please confirm it's intended (and document rationale if this repo should not use automated reviews).

Copilot · 2026-03-06T19:10:45Z

services/migrations.py

@@ -0,0 +1,41 @@
+from typing import Any, Dict, List, Optional


Optional is imported but not used, which will fail Ruff's unused-import check (F401). Remove the unused import.

Suggested change

from typing import Any, Dict, List, Optional

from typing import Any, Dict, List

Copilot · 2026-03-06T19:10:46Z

lambda_function.py

+        return {
+            "statusCode": error_type.code,
+            "headers": {
+                "Content-Type": "application/json",
+                "X-Request-ID": request_id,
+            },
+            "body": json.dumps(error_body, cls=DateTimeEncoder),
+        }


Error responses also omit CORS headers. If this Lambda is fronted by API Gateway and called from browsers, failed requests will be opaque due to CORS. Ensure the same CORS headers are present on error responses as on success responses (or configured in API Gateway).

Copilot · 2026-03-06T19:10:46Z

tests/test_lambda_function.py

+        manager = LambdaManager(self.event, self.mock_context)
+        self.assertEqual(manager.request_id, "test-request-id")
+        self.assertIsNone(manager.db_driver)
+
+    # TODO: Add more tests for error cases
+    def test_placeholder(self):
+        """A placeholder test that should be replaced with real test cases."""
+        self.assertTrue(True)
+


The placeholder test always passes and doesn't validate behavior; it reduces signal in CI and can hide real failures. Remove it and replace with concrete tests (e.g., unauthorized API key, missing secrets, NASA API failure, DB failure).

Suggested change

manager = LambdaManager(self.event, self.mock_context)

self.assertEqual(manager.request_id, "test-request-id")

self.assertIsNone(manager.db_driver)

# TODO: Add more tests for error cases

def test_placeholder(self):

"""A placeholder test that should be replaced with real test cases."""

self.assertTrue(True)

manager = LambdaManager(self.event, self.mock_context)

self.assertEqual(manager.request_id, "test-request-id")

self.assertIsNone(manager.db_driver)

# Error case: API key in the request does not match the secret -> unauthorized

@patch("lambda_function.SecretsManagerWrapper")

@patch("lambda_function.NasaService")

@patch("lambda_function.MysqlDriver")

@patch("lambda_function.MigrationRepository")

def test_lambda_handler_unauthorized_api_key(self, mock_repo, mock_db, mock_nasa, mock_secrets):

# Secrets contain a different key than the one provided in the event

mock_secrets_inst = mock_secrets.return_value

mock_secrets_inst.secrets = {

"LOGISTICS_API_GATEWAY_KEY": "real-key",

"NASA_API_KEY": "nasa-key",

"LAMBDA_LOG_LEVEL": "INFO",

}

# Other dependencies behave as normal

mock_nasa_inst = mock_nasa.return_value

mock_nasa_inst.get_astronomy_picture_of_the_day.return_value = {"url": "http://example.com"}

mock_repo_inst = mock_repo.return_value

mock_repo_inst.get_latest_migrations.return_value = [{"name": "001_init"}]

# Event still uses "test-key", which should not match "real-key"

response = lambda_handler(self.event, self.mock_context)

self.assertNotEqual(response.get("statusCode"), 200)

self.assertIn("error", str(response.get("body", "")).lower())

# Error case: required secrets are missing

@patch("lambda_function.SecretsManagerWrapper")

@patch("lambda_function.NasaService")

@patch("lambda_function.MysqlDriver")

@patch("lambda_function.MigrationRepository")

def test_lambda_handler_missing_secrets(self, mock_repo, mock_db, mock_nasa, mock_secrets):

mock_secrets_inst = mock_secrets.return_value

# Simulate missing configuration by exposing an empty secrets dict

mock_secrets_inst.secrets = {}

mock_nasa_inst = mock_nasa.return_value

mock_nasa_inst.get_astronomy_picture_of_the_day.return_value = {"url": "http://example.com"}

mock_repo_inst = mock_repo.return_value

mock_repo_inst.get_latest_migrations.return_value = [{"name": "001_init"}]

response = lambda_handler(self.event, self.mock_context)

self.assertNotEqual(response.get("statusCode"), 200)

self.assertIn("error", str(response.get("body", "")).lower())

# Error case: NASA service raises an exception

@patch("lambda_function.SecretsManagerWrapper")

@patch("lambda_function.NasaService")

@patch("lambda_function.MysqlDriver")

@patch("lambda_function.MigrationRepository")

def test_lambda_handler_nasa_failure(self, mock_repo, mock_db, mock_nasa, mock_secrets):

mock_secrets_inst = mock_secrets.return_value

mock_secrets_inst.secrets = {

"LOGISTICS_API_GATEWAY_KEY": "test-key",

"NASA_API_KEY": "nasa-key",

"LAMBDA_LOG_LEVEL": "INFO",

}

mock_nasa_inst = mock_nasa.return_value

mock_nasa_inst.get_astronomy_picture_of_the_day.side_effect = Exception("NASA API failure")

mock_repo_inst = mock_repo.return_value

mock_repo_inst.get_latest_migrations.return_value = [{"name": "001_init"}]

response = lambda_handler(self.event, self.mock_context)

self.assertNotEqual(response.get("statusCode"), 200)

self.assertIn("error", str(response.get("body", "")).lower())

# Error case: database / migration repository raises an exception

@patch("lambda_function.SecretsManagerWrapper")

@patch("lambda_function.NasaService")

@patch("lambda_function.MysqlDriver")

@patch("lambda_function.MigrationRepository")

def test_lambda_handler_db_failure(self, mock_repo, mock_db, mock_nasa, mock_secrets):

mock_secrets_inst = mock_secrets.return_value

mock_secrets_inst.secrets = {

"LOGISTICS_API_GATEWAY_KEY": "test-key",

"NASA_API_KEY": "nasa-key",

"LAMBDA_LOG_LEVEL": "INFO",

}

mock_nasa_inst = mock_nasa.return_value

mock_nasa_inst.get_astronomy_picture_of_the_day.return_value = {"url": "http://example.com"}

mock_repo_inst = mock_repo.return_value

mock_repo_inst.get_latest_migrations.side_effect = Exception("DB failure")

response = lambda_handler(self.event, self.mock_context)

self.assertNotEqual(response.get("statusCode"), 200)

self.assertIn("error", str(response.get("body", "")).lower())

Copilot · 2026-03-06T19:10:46Z

pyproject.toml

+    "pytest>=8.0.0",
 ]




pytest is added to the main runtime dependencies, which will be bundled into Lambda artifacts even though it's only needed for local testing. Consider moving it to a dev/optional dependency group to keep the production dependency set small.

Suggested change

"pytest>=8.0.0",

]

]

[project.optional-dependencies]

dev = [

"pytest>=8.0.0",

]

Copilot · 2026-03-06T19:10:46Z

lambda_function.py

 from requests.exceptions import RequestException
 from requests.models import HTTPError


HTTPError is imported from requests.models, which will raise an ImportError (HTTPError lives in requests.exceptions). Update the import so the Lambda can start and the exception handler works as intended.

Suggested change

from requests.exceptions import RequestException

from requests.models import HTTPError

from requests.exceptions import RequestException, HTTPError

Copilot · 2026-03-06T19:10:47Z

services/nasa.py

+from requests.exceptions import RequestException
+from requests.models import HTTPError


HTTPError is imported from requests.models, which is incorrect (HTTPError is in requests.exceptions). This will fail at import-time and prevent the service from running.

Suggested change

from requests.exceptions import RequestException

from requests.models import HTTPError

from requests.exceptions import HTTPError, RequestException

Refac NASA service and test impl

cea1c0a

simlal requested a review from Copilot March 6, 2026 19:06

Copilot started reviewing on behalf of simlal March 6, 2026 19:06 View session

Copilot AI reviewed Mar 6, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refac NASA service and test impl#8

Refac NASA service and test impl#8
simlal wants to merge 1 commit intomainfrom
gemini-improvements-test-ide-plugins-copilot

simlal commented Mar 6, 2026

Uh oh!

coderabbitai bot commented Mar 6, 2026

Review skipped

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -0,0 +1,41 @@
		from typing import Any, Dict, List, Optional

	from typing import Any, Dict, List, Optional
	from typing import Any, Dict, List

-        manager = LambdaManager(self.event, self.mock_context)
-        self.assertEqual(manager.request_id, "test-request-id")
-        self.assertIsNone(manager.db_driver)
-    # TODO: Add more tests for error cases
-    def test_placeholder(self):
-        """A placeholder test that should be replaced with real test cases."""
-        self.assertTrue(True)
+    manager = LambdaManager(self.event, self.mock_context)
+    self.assertEqual(manager.request_id, "test-request-id")
+    self.assertIsNone(manager.db_driver)
+    # Error case: API key in the request does not match the secret -> unauthorized
+    @patch("lambda_function.SecretsManagerWrapper")
+    @patch("lambda_function.NasaService")
+    @patch("lambda_function.MysqlDriver")
+    @patch("lambda_function.MigrationRepository")
+    def test_lambda_handler_unauthorized_api_key(self, mock_repo, mock_db, mock_nasa, mock_secrets):
+    # Secrets contain a different key than the one provided in the event
+    mock_secrets_inst = mock_secrets.return_value
+    mock_secrets_inst.secrets = {
+        "LOGISTICS_API_GATEWAY_KEY": "real-key",
+        "NASA_API_KEY": "nasa-key",
+        "LAMBDA_LOG_LEVEL": "INFO",
+    }
+    # Other dependencies behave as normal
+    mock_nasa_inst = mock_nasa.return_value
+    mock_nasa_inst.get_astronomy_picture_of_the_day.return_value = {"url": "http://example.com"}
+    mock_repo_inst = mock_repo.return_value
+    mock_repo_inst.get_latest_migrations.return_value = [{"name": "001_init"}]
+    # Event still uses "test-key", which should not match "real-key"
+    response = lambda_handler(self.event, self.mock_context)
+    self.assertNotEqual(response.get("statusCode"), 200)
+    self.assertIn("error", str(response.get("body", "")).lower())
+    # Error case: required secrets are missing
+    @patch("lambda_function.SecretsManagerWrapper")
+    @patch("lambda_function.NasaService")
+    @patch("lambda_function.MysqlDriver")
+    @patch("lambda_function.MigrationRepository")
+    def test_lambda_handler_missing_secrets(self, mock_repo, mock_db, mock_nasa, mock_secrets):
+    mock_secrets_inst = mock_secrets.return_value
+    # Simulate missing configuration by exposing an empty secrets dict
+    mock_secrets_inst.secrets = {}
+    mock_nasa_inst = mock_nasa.return_value
+    mock_nasa_inst.get_astronomy_picture_of_the_day.return_value = {"url": "http://example.com"}
+    mock_repo_inst = mock_repo.return_value
+    mock_repo_inst.get_latest_migrations.return_value = [{"name": "001_init"}]
+    response = lambda_handler(self.event, self.mock_context)
+    self.assertNotEqual(response.get("statusCode"), 200)
+    self.assertIn("error", str(response.get("body", "")).lower())
+    # Error case: NASA service raises an exception
+    @patch("lambda_function.SecretsManagerWrapper")
+    @patch("lambda_function.NasaService")
+    @patch("lambda_function.MysqlDriver")
+    @patch("lambda_function.MigrationRepository")
+    def test_lambda_handler_nasa_failure(self, mock_repo, mock_db, mock_nasa, mock_secrets):
+    mock_secrets_inst = mock_secrets.return_value
+    mock_secrets_inst.secrets = {
+        "LOGISTICS_API_GATEWAY_KEY": "test-key",
+        "NASA_API_KEY": "nasa-key",
+        "LAMBDA_LOG_LEVEL": "INFO",
+    }
+    mock_nasa_inst = mock_nasa.return_value
+    mock_nasa_inst.get_astronomy_picture_of_the_day.side_effect = Exception("NASA API failure")
+    mock_repo_inst = mock_repo.return_value
+    mock_repo_inst.get_latest_migrations.return_value = [{"name": "001_init"}]
+    response = lambda_handler(self.event, self.mock_context)
+    self.assertNotEqual(response.get("statusCode"), 200)
+    self.assertIn("error", str(response.get("body", "")).lower())
+    # Error case: database / migration repository raises an exception
+    @patch("lambda_function.SecretsManagerWrapper")
+    @patch("lambda_function.NasaService")
+    @patch("lambda_function.MysqlDriver")
+    @patch("lambda_function.MigrationRepository")
+    def test_lambda_handler_db_failure(self, mock_repo, mock_db, mock_nasa, mock_secrets):
+    mock_secrets_inst = mock_secrets.return_value
+    mock_secrets_inst.secrets = {
+        "LOGISTICS_API_GATEWAY_KEY": "test-key",
+        "NASA_API_KEY": "nasa-key",
+        "LAMBDA_LOG_LEVEL": "INFO",
+    }
+    mock_nasa_inst = mock_nasa.return_value
+    mock_nasa_inst.get_astronomy_picture_of_the_day.return_value = {"url": "http://example.com"}
+    mock_repo_inst = mock_repo.return_value
+    mock_repo_inst.get_latest_migrations.side_effect = Exception("DB failure")
+    response = lambda_handler(self.event, self.mock_context)
+    self.assertNotEqual(response.get("statusCode"), 200)
+    self.assertIn("error", str(response.get("body", "")).lower())

		from requests.exceptions import RequestException
		from requests.models import HTTPError

	from requests.exceptions import RequestException
	from requests.models import HTTPError
	from requests.exceptions import HTTPError, RequestException

		[config]
		disable_auto_feedback = true

Conversation

simlal commented Mar 6, 2026

Uh oh!

coderabbitai bot commented Mar 6, 2026

Review skipped

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants