-
Notifications
You must be signed in to change notification settings - Fork 112
Add endpoint to get all players' names #99
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.
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
base: main
Are you sure you want to change the base?
Conversation
Add endpoint to fetch all players' names. * Add a new endpoint `get_all_players_names` in `app/api/endpoints/players.py`. * Create a new method `get_all_players_names` in `app/services/players/search.py` to fetch all players' names. * Update the `PlayerSearch` schema in `app/schemas/players/search.py` to include only the `name` field. * Add a new test case in `tests/players/test_players_search.py` to test the `get_all_players_names` method. * Add `main.py` file with a simple print statement. * Update `pyproject.toml` to reflect the new project structure and dependencies. * Add `uv.lock` file with the project dependencies. --- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/felipeall/transfermarkt-api?shareId=XXXX-XXXX-XXXX-XXXX).
WalkthroughThis update adds a new API endpoint that returns a list of player names. It introduces a new function in the players endpoint that leverages an additional method in the player search service to retrieve names. The player search schema is simplified by removing several classes and attributes. A basic main entry point is added for startup, and project metadata is restructured in the configuration file. A new test case verifies that the player names are correctly retrieved. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant PlayerSearchService
participant DataFetcher
Client->>API: GET /all_players_names
API->>PlayerSearchService: get_all_players_names()
PlayerSearchService->>DataFetcher: search_players()
DataFetcher-->>PlayerSearchService: search results
PlayerSearchService-->>API: list of player names
API-->>Client: JSON response with player names
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pyproject.toml (2)
1-6: Project configuration updated but description needs attentionThe project structure has been updated from Poetry-specific format to standard Python project format, which is a good modernization step. However, the description has been changed to a placeholder "Add your description here" instead of the meaningful previous description.
-description = "Add your description here" +description = "API service to get data from Transfermarkt"
7-43: Dependencies explicitly pinned with consistent version constraintsThe dependencies section has been completely restructured to use explicit version pinning with consistent conditional expressions based on Python version. This makes dependency management more predictable and helps prevent "works on my machine" issues.
However, the explicit pinning with such specific version constraints might make maintenance more challenging as updates will require manual intervention.
Consider using a requirements.txt file for exact pinning during deployment while keeping broader version constraints in pyproject.toml:
dependencies = [ - "annotated-types==0.7.0 ; python_full_version >= '3.9' and python_full_version < '4.0'", + "annotated-types>=0.7.0", - "anyio==4.7.0 ; python_full_version >= '3.9' and python_full_version < '4.0'", + "anyio>=4.7.0", # Similar changes for other dependencies ]This approach maintains compatibility while allowing for minor updates that don't break your code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
app/api/endpoints/players.py(1 hunks)app/schemas/players/search.py(1 hunks)app/services/players/search.py(1 hunks)main.py(1 hunks)pyproject.toml(1 hunks)tests/players/test_players_search.py(1 hunks)
🔇 Additional comments (7)
main.py (1)
1-6: Clean, simple entry point implementation.This provides a basic entry point for running the application directly, following standard Python practices.
app/api/endpoints/players.py (1)
74-78:✅ Verification successful
Good implementation of the new endpoint.
The endpoint follows the established pattern in the codebase and correctly implements the functionality to retrieve all player names. The return type annotation
list[str]properly matches the expected response structure.However, I notice that
TransfermarktPlayerSearchis initialized without any parameters, while other instances in this file provide a query parameter. Let's verify this works as expected:
🏁 Script executed:
#!/bin/bash # Check if TransfermarktPlayerSearch can be initialized without query parameter rg -A 3 -B 3 "def __post_init__" app/services/players/search.pyLength of output: 323
I'll run a search for the
TransfermarktPlayerSearchclass definition to verify how its constructor and default parameters are defined. Please hold on while I gather this information.
🏁 Script executed:
#!/bin/bash # Search for the TransfermarktPlayerSearch class declaration to check its __init__ signature and default parameters. rg "class TransfermarktPlayerSearch" -n app/services/players/search.pyLength of output: 127
Endpoint Functionality and Query Parameter Clarification
The new endpoint inapp/api/endpoints/players.py(lines 74–78) is well implemented. Our verification shows that while the endpoint instantiatesTransfermarktPlayerSearchwithout explicitly passing a query, its__post_init__method usesself.queryappropriately—which implies that the class is designed to handle a missing query (e.g., via a default or alternative logic). This design differs intentionally from other instances where a query is provided, as the endpoint’s purpose is to retrieve all player names. Overall, no issues were found.app/services/players/search.py (1)
87-96:✅ Verification successful
Well-documented method with clear implementation.
The new method is properly documented with comprehensive docstrings and has a clear implementation. It reuses the existing
search_players()method and extracts just the player names, following the single responsibility principle.However, I'm curious about how this works without a query parameter, as the
search_players()method seems to useself.queryin its implementation. Let's verify this:
🏁 Script executed:
#!/bin/bash # Check how search_players handles None query rg -A 4 "self.response\[\"query\"\] = self.query" app/services/players/search.pyLength of output: 334
Query Parameter Usage Confirmed
The verification confirms that the
search_players()method relies on the instance attributeself.query—assigned within its implementation—to build the search response. Therefore, the absence of an explicit query parameter when calling it fromget_all_players_names()is intentional and appropriate given the design.No changes are required.
tests/players/test_players_search.py (1)
57-65: Good test coverage for the new functionality.The test properly verifies that the
get_all_players_names()method returns a non-empty list of non-empty strings. The test follows the pattern of other tests in the file and uses appropriate assertions.app/schemas/players/search.py (1)
1-6: Schema simplified to support the new endpointThe PlayerSearch model has been significantly refactored to only include the
namefield, which aligns perfectly with the PR objective of creating an endpoint that returns only player names. The removal of other fields (query, pageNumber, lastPageNumber, results) and related classes (PlayerSearchClub, PlayerSearchResult) streamlines the response data structure.This change appropriately eliminates the AuditMixin inheritance as tracking creation/modification timestamps isn't necessary for this simple data structure.
pyproject.toml (2)
3-3: Version downgraded from 3.0.0 to 0.1.0The project version has been significantly downgraded from 3.0.0 to 0.1.0. This is unusual for an established project and may cause compatibility issues for users.
Please verify if this version downgrade was intentional. If this is a new module or component, the version makes sense, but if it's intended to replace the existing package, maintaining version continuity is important for semantic versioning principles.
6-6: Python requirement increased to >=3.12The minimum Python version requirement has been increased from 3.9 to 3.12, which is a significant change. Python 3.12 is relatively recent, and this change may limit the usability of the package for users who haven't upgraded yet.
Please confirm that this constraint is necessary. If specific 3.12 features are required, this change is appropriate; otherwise, maintaining compatibility with Python 3.9 would provide better backward compatibility.
Add endpoint to fetch all players' names.
get_all_players_namesinapp/api/endpoints/players.py.get_all_players_namesinapp/services/players/search.pyto fetch all players' names.PlayerSearchschema inapp/schemas/players/search.pyto include only thenamefield.tests/players/test_players_search.pyto test theget_all_players_namesmethod.main.pyfile with a simple print statement.pyproject.tomlto reflect the new project structure and dependencies.uv.lockfile with the project dependencies.For more details, open the Copilot Workspace session.
Summary by CodeRabbit
New Features
Refactor
Tests