Skip to content

Comments

Add tournament team route to sendou proxy#63

Merged
cesaregarza merged 1 commit intomainfrom
feature/sendou-data-workaround-2
Jan 3, 2026
Merged

Add tournament team route to sendou proxy#63
cesaregarza merged 1 commit intomainfrom
feature/sendou-data-workaround-2

Conversation

@cesaregarza
Copy link
Owner

This commit introduces a new endpoint to fetch tournament team data from sendou.ink, enhancing the API's capabilities. The new route proxies requests to the sendou.ink turbo-stream endpoint and returns the data in plain JSON format. Error handling has been implemented to manage HTTP and decoding issues, ensuring robust functionality.

This commit introduces a new endpoint to fetch tournament team data from sendou.ink, enhancing the API's capabilities. The new route proxies requests to the sendou.ink turbo-stream endpoint and returns the data in plain JSON format. Error handling has been implemented to manage HTTP and decoding issues, ensuring robust functionality.
@claude
Copy link

claude bot commented Jan 3, 2026

Pull Request Review: Add tournament team route to sendou proxy

Summary

This PR adds a new endpoint /api/sendou/to/{tournament_id}/teams/{team_id} that mirrors the existing tournament match endpoint. The implementation follows established patterns in the codebase and looks solid overall.


✅ Strengths

  1. Excellent code consistency: The new get_tournament_team function follows the exact same pattern as get_tournament_match, maintaining consistency in error handling, logging, and structure.

  2. Proper error handling: All error cases are covered:

    • HTTP errors from sendou.ink (with appropriate status code passthrough)
    • Network/request errors (502 Bad Gateway)
    • Decoding errors (502 Bad Gateway)
  3. Good logging: Appropriate log levels used (warning for HTTP errors, error for failures).

  4. Clear documentation: Docstring, summary, and description provide clear information about what the endpoint does.


🔍 Code Quality Observations

No issues found - The code follows best practices:

  • Type hints are used correctly (Dict[str, Any])
  • FastAPI Path parameters with descriptions
  • Async/await properly implemented
  • Follows the repository's formatting standards (Black/isort at 80 char line length)

🐛 Potential Issues

None identified - The implementation is straightforward and follows proven patterns from get_tournament_match.


⚡ Performance Considerations

The implementation looks good from a performance standpoint:

  • Uses httpx.AsyncClient for non-blocking I/O
  • Proper timeout configured (REQUEST_TIMEOUT = 10.0s)
  • No unnecessary processing or blocking operations

Minor observation: Both endpoints create a new AsyncClient on each request. This is fine, but if these endpoints see heavy traffic, consider using a shared client instance with connection pooling for better performance.


🔒 Security Concerns

No security issues identified:

  • No user input is directly interpolated into dangerous operations
  • URL construction uses f-strings with integer-validated path parameters (no injection risk)
  • Error messages don't leak sensitive information
  • Appropriate status codes returned

🧪 Test Coverage

Missing: There are no tests for the sendou proxy routes at all.

Recommendation: Consider adding tests similar to other routes in the project. Example test cases:

  • Successful team data retrieval
  • HTTP 404 from sendou.ink (team not found)
  • Network errors
  • Invalid turbo-stream response
  • Route key extraction

A basic test structure would be:

# tests/sendou/test_sendou_proxy.py
import pytest
from unittest.mock import AsyncMock, patch

@pytest.mark.asyncio
async def test_get_tournament_team_success(client):
    # Mock successful response from sendou.ink
    ...

@pytest.mark.asyncio  
async def test_get_tournament_team_not_found(client):
    # Mock 404 response
    ...

Note: Since this mirrors get_tournament_match which also lacks tests, this isn't a blocker for this specific PR, but it would be valuable to add test coverage for both endpoints.


📋 Summary

Overall Assessment: ✅ Approve with minor suggestions

This is a clean, well-implemented addition that properly extends the sendou proxy functionality. The code quality is high and follows established patterns.

Action Items (optional, can be addressed in future PRs):

  1. Consider adding test coverage for sendou proxy routes
  2. If traffic increases, consider connection pooling for httpx client

Files changed:

  • src/fast_api_app/routes/sendou_proxy.py:29 - Added ROUTE_KEY_TOURNAMENT_TEAM constant
  • src/fast_api_app/routes/sendou_proxy.py:111-156 - Added get_tournament_team endpoint

Great work maintaining code consistency! 🎉

@cesaregarza cesaregarza merged commit 074c6ff into main Jan 3, 2026
7 checks passed
@cesaregarza cesaregarza deleted the feature/sendou-data-workaround-2 branch January 3, 2026 04:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant