Architecture alignment #37
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR aligns the data-model, data-model-pipeline, and platform-deploy repositories with the ADR docs by implementing architecture alignment todos, fixing pre-commit issues, and adding a devseed-staging namespace sandbox.
Key changes include:
- Comprehensive type hint updates replacing legacy type annotations with modern alternatives
- New pipeline integration module with payload schema validation and shared data models
- Addition of STAC Transactions client for API interactions
Reviewed Changes
Copilot reviewed 24 out of 25 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/eopf_geozarr/pipeline/ | New pipeline orchestration module with shared payload schema, models, and runner |
| src/eopf_geozarr/data_api/transactions.py | New STAC Transactions API client for collection/item management |
| src/eopf_geozarr/validator.py | New GeoZarr store validation functionality |
| src/eopf_geozarr/conversion/*.py | Type hint modernization replacing Dict/List with dict/list annotations |
| src/eopf_geozarr/cli.py | Enhanced CLI with metrics output support and improved error handling |
| tests/ | Comprehensive test coverage for new pipeline, transactions, and validation modules |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| help=( | ||
| "Groups that need CRS information added on best-effort basis " | ||
| "(e.g., /conditions/geometry)" | ||
| ), |
There was a problem hiding this comment.
The help string appears to be incorrectly placed in the middle of a CSS style definition. This code block seems to be misplaced and should be moved to the appropriate argument parser section.
| help=( | |
| "Groups that need CRS information added on best-effort basis " | |
| "(e.g., /conditions/geometry)" | |
| ), |
Copilot uses AI. Check for mistakes.
| for key, group in geozarr_groups.items(): | ||
| if group.rio.crs: | ||
| crs_string: str = group.rio.crs.to_string() | ||
| print(f"Inferred reference CRS from measurements: {crs_string}") |
There was a problem hiding this comment.
This debug print statement should use proper logging instead of print() to maintain consistent logging patterns throughout the codebase.
Copilot uses AI. Check for mistakes.
| output_path, group_name.lstrip("/"), written_var | ||
| ) | ||
| ) | ||
| except Exception as exc: # noqa: PERF203 - legacy retry policy |
There was a problem hiding this comment.
The noqa comment references PERF203 which seems incorrect for this context. The comment should explain why catching a broad Exception is necessary here, rather than referencing an unrelated performance rule.
| except Exception as exc: # noqa: PERF203 - legacy retry policy | |
| except Exception as exc: # Catching broad Exception is necessary to ensure all errors during write are retried. |
Copilot uses AI. Check for mistakes.
| # Add CRS information if needed | ||
| if crs_groups and current_group_path in crs_groups: | ||
| print(f"Adding CRS information for group '{current_group_path}'") | ||
| print(f"Adding CRS information to group: {current_group_path}") |
There was a problem hiding this comment.
This debug print statement should use proper logging instead of print() to maintain consistent logging patterns throughout the codebase.
Copilot uses AI. Check for mistakes.
| local filesystem or directly to `s3://` destinations. The payload helpers expose the same field so Argo | ||
| Workflow templates and RabbitMQ messages stay aligned. | ||
| ``` | ||
| ``` |
There was a problem hiding this comment.
There are two consecutive closing code fence blocks (```) on lines 107-108, which will cause markdown parsing errors. Remove one of the closing code fences.
| ``` |
Copilot uses AI. Check for mistakes.
Aligns data-model, data-model-pipeline and platform-deploy with the ADR docs.
Refs: developmentseed/sentinel-zarr-explorer-coordination#117
Doc: https://github.com/EOPF-Explorer/data-model-pipeline/blob/feat/shared-payload-publisher/docs/architecture-alignment-todos.md