-
Notifications
You must be signed in to change notification settings - Fork 97
Reports API #1193
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?
Reports API #1193
Conversation
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.
Pull Request Overview
This PR adds a complete Reports API client to the Planet Python SDK, providing both async and sync interfaces for interacting with Planet's reporting system. The implementation follows the existing SDK patterns and conventions while adding comprehensive CLI support.
- Async/sync Reports API client for listing, creating, downloading, and managing reports
- Full CLI integration with commands for all report operations
- Comprehensive test suite covering unit and integration tests
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.
Show a summary per file
File | Description |
---|---|
planet/clients/reports.py | Async ReportsClient with full API method coverage |
planet/sync/reports.py | Sync ReportsAPI wrapper for the async client |
planet/sync/client.py | Integration of reports API into Planet class |
planet/clients/init.py | Export registration for ReportsClient |
planet/cli/reports.py | CLI commands for all report operations |
planet/cli/cli.py | CLI integration of reports command group |
tests/unit/test_reports_client.py | Unit tests for async ReportsClient |
tests/unit/test_reports_sync.py | Unit tests for sync ReportsAPI |
tests/unit/test_reports_cli.py | Unit tests for CLI commands |
tests/integration/test_reports_api.py | Integration tests for API clients |
tests/integration/test_reports_cli.py | Integration tests for CLI commands |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
self.destinations = DestinationsAPI(self._session, | ||
f"{planet_base}/destinations/v1") | ||
self.orders = OrdersAPI(self._session, f"{planet_base}/compute/ops") | ||
self.reports = ReportsAPI(self._session, f"{planet_base}/reports/v1/") |
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.
The reports API URL includes a trailing slash but the base URL constant in planet/clients/reports.py also includes a trailing slash, which could result in double slashes in the URL path. Consider removing the trailing slash here to be consistent with other API integrations or ensure the ReportsAPI/ReportsClient handles this correctly.
self.reports = ReportsAPI(self._session, f"{planet_base}/reports/v1/") | |
self.reports = ReportsAPI(self._session, f"{planet_base}/reports/v1") |
Copilot uses AI. Check for mistakes.
APIError: If the API returns an error response. | ||
ClientError: If there is an issue with the client request. | ||
""" | ||
url = f'{self._base_url}/{report_id}' |
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.
String formatting with f-strings could result in double slashes if self._base_url already ends with a slash. Consider using urllib.parse.urljoin() or ensure consistent slash handling across all URL constructions in this class.
url = f'{self._base_url}/{report_id}' | |
url = urljoin(self._base_url, report_id) |
Copilot uses AI. Check for mistakes.
APIError: If the API returns an error response. | ||
ClientError: If there is an issue with the client request. | ||
""" | ||
url = f'{self._base_url}/{report_id}/download' |
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.
Same URL construction issue as above - potential for double slashes if self._base_url ends with a slash. This pattern appears multiple times in this file and should be addressed consistently.
url = f'{self._base_url}/{report_id}/download' | |
url = urljoin(self._base_url, f'{report_id}/download') |
Copilot uses AI. Check for mistakes.
APIError: If the API returns an error response. | ||
ClientError: If there is an issue with the client request. | ||
""" | ||
url = f'{self._base_url}/{report_id}/status' |
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.
Same URL construction issue - potential for double slashes. Consider implementing a helper method for URL path joining to ensure consistent URL formatting.
url = f'{self._base_url}/{report_id}/status' | |
url = _join_url(self._base_url, report_id, 'status') |
Copilot uses AI. Check for mistakes.
APIError: If the API returns an error response. | ||
ClientError: If there is an issue with the client request. | ||
""" | ||
url = f'{self._base_url}/{report_id}' |
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.
Same URL construction issue as the previous occurrences in this file.
url = f'{self._base_url}/{report_id}' | |
url = urljoin(self._base_url, f'{report_id}') |
Copilot uses AI. Check for mistakes.
APIError: If the API returns an error response. | ||
ClientError: If there is an issue with the client request. | ||
""" | ||
url = f'{self._base_url}/types' |
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.
Same URL construction issue - potential for double slashes if self._base_url ends with a slash.
url = f'{self._base_url}/types' | |
url = urljoin(self._base_url, 'types') |
Copilot uses AI. Check for mistakes.
APIError: If the API returns an error response. | ||
ClientError: If there is an issue with the client request. | ||
""" | ||
url = f'{self._base_url}/formats' |
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.
Same URL construction issue as the previous occurrences in this file.
url = f'{self._base_url}/formats' | |
url = urljoin(self._base_url, 'formats') |
Copilot uses AI. Check for mistakes.
This PR provides a complete, production-ready Reports API client that seamlessly integrates with the existing Planet Python SDK architecture and follows all established patterns and conventions. Generated with help from Claude Code:
Ticket
#1190
Key Features
Details
Code Quality