Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
32 changes: 32 additions & 0 deletions app.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#!/usr/bin/env python3
"""
Simple example app demonstrating REST API functions.

This module provides a simple HTTP server with REST API endpoints.

REST API functions are now in rest_api.py (refactored from TODO).
Copy link
Collaborator

Choose a reason for hiding this comment

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

🟠 Important: The comment claims this is "refactored from TODO", but this PR creates new files rather than modifying existing ones. Where was the original app.py with the TODO comment at line 7? This appears to be adding new code, not refactoring.

"""

from rest_api import get_health, get_status, get_version, create_item, get_items
Copy link
Collaborator

Choose a reason for hiding this comment

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

🟡 Suggestion: The import is incomplete. The rest_api.py module defines get_item() and delete_item(), but they are not imported here. Consider adding them if they will be used:

Suggested change
from rest_api import get_health, get_status, get_version, create_item, get_items
from rest_api import get_health, get_status, get_version, create_item, get_items, get_item, delete_item



def main():
"""Main application entry point."""
print("REST API Application")
print(f"Health: {get_health()}")
print(f"Status: {get_status()}")
print(f"Version: {get_version()}")

# Demo CRUD operations
items = get_items()
print(f"Current items: {items}")

new_item = create_item("test_item", {"value": 42})
print(f"Created item: {new_item}")

items = get_items()
print(f"Updated items: {items}")


if __name__ == "__main__":
main()
93 changes: 93 additions & 0 deletions rest_api.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
"""
REST API functions for the application.

This module contains all REST API-related functions that were extracted
from app.py for better code organization and maintainability.
Comment on lines +1 to +5
Copy link
Collaborator

Choose a reason for hiding this comment

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

🔴 Critical: Misleading naming - these are NOT REST API functions. There is no web framework (Flask, FastAPI, etc.), no HTTP handlers, no routing. These are just plain Python functions with in-memory storage. This will confuse developers expecting actual REST endpoints. Consider renaming to something like data_operations.py or item_storage.py.

"""

# In-memory storage for demo purposes
_items: dict = {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

🟠 Important: Using a module-level mutable dictionary as storage is problematic:

  1. Not thread-safe (race conditions in concurrent access)
  2. Data persists across imports, which can cause unexpected behavior in tests
  3. No data validation or constraints

Consider either:

  • Adding explicit warnings that this is for demo/testing only
  • Using a proper data store or at least wrapping access in a class with locks



def get_health() -> dict:
"""Get health status of the API.

Returns:
dict: Health status information.
"""
return {"status": "healthy", "service": "rest-api"}


def get_status() -> dict:
"""Get current status of the API server.

Returns:
dict: Server status information.
"""
return {
"running": True,
"uptime": "unknown",
"connections": 0,
}


def get_version() -> dict:
"""Get API version information.

Returns:
dict: Version information.
"""
return {
"version": "1.0.0",
"api_version": "v1",
}


def create_item(name: str, data: dict) -> dict:
"""Create a new item in the API.

Args:
name: Name/identifier for the item.
data: Data to store for the item.

Returns:
dict: Created item information.
"""
_items[name] = data
return {"name": name, "data": data, "created": True}
Comment on lines +45 to +57
Copy link
Collaborator

Choose a reason for hiding this comment

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

🟠 Important: Missing input validation. The function should validate:

  • name is not empty/None
  • name is a valid string
  • data is a dictionary
  • Whether overwriting existing items is allowed
Suggested change
def create_item(name: str, data: dict) -> dict:
"""Create a new item in the API.
Args:
name: Name/identifier for the item.
data: Data to store for the item.
Returns:
dict: Created item information.
"""
_items[name] = data
return {"name": name, "data": data, "created": True}
def create_item(name: str, data: dict) -> dict:
"""Create a new item in the API.
Args:
name: Name/identifier for the item.
data: Data to store for the item.
Returns:
dict: Created item information.
Raises:
ValueError: If name is empty or data is not a dict.
"""
if not name or not isinstance(name, str):
raise ValueError("name must be a non-empty string")
if not isinstance(data, dict):
raise TypeError("data must be a dictionary")
_items[name] = data
return {"name": name, "data": data, "created": True}



def get_items() -> dict:
"""Get all items from the API.

Returns:
dict: All stored items.
"""
return dict(_items)


def get_item(name: str) -> dict | None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

🟡 Suggestion: The union syntax dict | None requires Python 3.10+. For better compatibility, consider using Optional[dict] from typing:

Suggested change
def get_item(name: str) -> dict | None:
def get_item(name: str) -> Optional[dict]:

(Add from typing import Optional at the top of the file)

"""Get a specific item by name.

Args:
name: Name of the item to retrieve.

Returns:
dict | None: Item data if found, None otherwise.
"""
return _items.get(name)


def delete_item(name: str) -> bool:
Copy link
Collaborator

Choose a reason for hiding this comment

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

🟡 Suggestion: This function is defined but never imported or used in app.py. Consider adding a demo of the delete functionality in the main() function to show the complete CRUD operations.

"""Delete an item by name.

Args:
name: Name of the item to delete.

Returns:
bool: True if deleted, False if not found.
"""
if name in _items:
del _items[name]
return True
return False
Loading