feat(corpora_pm): start issues interface with github

skyl commented 1 week ago

PR Type

Enhancement, Tests

Description

Introduced an abstract interface AbstractIssueTracker for issue tracking systems, defining methods for managing issues.
Implemented GitHubIssueTracker, a concrete class for interacting with GitHub's issue tracking via API.
Added comprehensive unit tests for the GitHubIssueTracker class using unittest framework.
Updated docker-compose.yaml to include GITHUB_TOKEN for GitHub API authentication.

Changes walkthrough 📝

Relevant files

Enhancement

abstract.py `Define abstract interface for issue tracking systems` py/packages/corpora_pm/abstract.py Introduced `Issue` class to represent issues. Defined `AbstractIssueTracker` as an interface for issue tracking. Added abstract methods for listing, retrieving, creating, updating issues, and adding comments.	+132/-0
pm.py `Implement GitHub issue tracker with API integration` py/packages/corpora_pm/providers/github/pm.py Implemented `GitHubIssueTracker` class extending `AbstractIssueTracker`. Added methods to interact with GitHub API for issue management. Utilized requests library for HTTP requests.	+113/-0

Tests

test_pm.py

Add unit tests for GitHub issue tracker

py/packages/corpora_pm/providers/github/test_pm.py

Added unit tests for GitHubIssueTracker methods.

Used unittest and unittest.mock for testing.

Tested issue listing, retrieval, creation, updating, and commenting.

+134/-0

Configuration changes

docker-compose.yaml

Update docker-compose for GitHub token configuration

docker-compose.yaml

Added GITHUB_TOKEN environment variable.

Updated service configuration for GitHub integration.

+2/-0

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

github-actions[bot] commented 1 week ago

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 Security concerns

Sensitive information exposure:
The use of `GITHUB_TOKEN` in the `docker-compose.yaml` file and in the code requires careful handling to avoid exposure. Ensure that the token is not logged or exposed in any way.

⚡ Recommended focus areas for review

Security Concern
The `_request` method uses a GitHub token for authentication. Ensure that the token is securely managed and not exposed in logs or error messages. Error Handling
The `_request` method raises an exception if the request fails. Consider adding more robust error handling to manage different types of HTTP errors gracefully.

github-actions[bot] commented 1 week ago

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Best practice	Add exception handling for network requests to improve error management ___ Handle potential exceptions from the `requests` library in the `_request` method to improve error handling and robustness. [py/packages/corpora_pm/providers/github/pm.py [28-29]](https://github.com/skyl/corpora/pull/18/files#diff-c7052c1583cde5ae2e23429e463a41f1dc4b62ac1ef77ecfe0643796e2a4be87R28-R29) ```diff -response = requests.request(method, url, headers=headers, json=data) -response.raise_for_status() +try: + response = requests.request(method, url, headers=headers, json=data) + response.raise_for_status() +except requests.exceptions.RequestException as e: + # Handle exception ``` Suggestion importance[1-10]: 8 Why: Adding exception handling for network requests is a best practice that enhances the robustness of the code by allowing it to gracefully handle potential network-related errors, improving the overall reliability of the application.	8
Possible issue	Add a check to ensure the GitHub token is not None before using it in requests ___ Ensure that the `token` is not `None` before using it in the `_request` method to avoid authentication errors. [py/packages/corpora_pm/providers/github/pm.py [26]](https://github.com/skyl/corpora/pull/18/files#diff-c7052c1583cde5ae2e23429e463a41f1dc4b62ac1ef77ecfe0643796e2a4be87R26-R26) ```diff -headers = {"Authorization": f"token {self.token}"} +headers = {"Authorization": f"token {self.token}"} if self.token else {} ``` Suggestion importance[1-10]: 7 Why: This suggestion adds a check to ensure that the GitHub token is not None before using it, which can prevent authentication errors. It is a practical improvement for robustness, especially in environments where the token might not be set.	7
Possible bug	Validate the state parameter in the list_issues method to ensure it has a valid value ___ Validate the `state` parameter in the `list_issues` method to ensure it only accepts valid values like "open", "closed", or "all". [py/packages/corpora_pm/providers/github/pm.py [32]](https://github.com/skyl/corpora/pull/18/files#diff-c7052c1583cde5ae2e23429e463a41f1dc4b62ac1ef77ecfe0643796e2a4be87R32-R32) ```diff def list_issues(self, repo: str, state: str = "open") -> List[Issue]: + if state not in ["open", "closed", "all"]: + raise ValueError("Invalid state value") ``` Suggestion importance[1-10]: 6 Why: This suggestion improves the method by validating the state parameter, ensuring it only accepts valid values. This prevents potential bugs from invalid input and aligns with the method's expected behavior.	6
Enhancement	Add assertions to verify the correct HTTP method and endpoint in test cases ___ Add assertions to verify the correct HTTP method and endpoint are used in the `test_create_issue` and `test_update_issue` methods to ensure the requests are properly constructed. [py/packages/corpora_pm/providers/github/test_pm.py [64]](https://github.com/skyl/corpora/pull/18/files#diff-c0427897f21d52151f2e13c40dc971c978ebb9bded1a748b48c7b5cbf4347a6cR64-R64) ```diff mock_request.return_value = mock_response +mock_request.assert_called_once_with("POST", ...) ``` Suggestion importance[1-10]: 5 Why: Adding assertions in test cases to verify the correct HTTP method and endpoint ensures that the requests are constructed as expected, enhancing the reliability and accuracy of the tests.	5

skyl / corpora