TRADE-738: Updates for deploying CASPER to UAT #832
Conversation
kecunning
requested review from
chris-durbin,
indiejames and
ygliuvt
as code owners
📝 WalkthroughWalkthroughAdds a new public Casper-backed service Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Harmony as Harmony
participant CMR as QUERY_CMR_IMAGE
participant Provider as Provider_Image
participant Casper as CASPER_IMAGE
participant Storage as Storage
Client->>Harmony: Submit job (service: asdc/casper or request output_format=text/csv)
Harmony->>CMR: QUERY_CMR_IMAGE (discover inputs)
CMR-->>Harmony: image list / metadata
Harmony->>Provider: Invoke provider image(s) processing
Provider-->>Harmony: produce artifact(s)
alt output_format == text/csv
Harmony->>Casper: Invoke CASPER_IMAGE (NetCDF → CSV conversion/concat)
Casper-->>Storage: write final CSV
else other formats
Harmony->>Storage: write final artifact(s)
end
Storage-->>Client: deliver/notify completed output
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @config/services-uat.yml:
- Around line 302-304: The service description for l2-subsetter-concise-casper
contains typos; update the description string (the value for the description key
in the l2-subsetter-concise-casper service block) to read "Service that converts
a NetCDF file to one or more CSV files." so "and" → "a" and "CVS" → "CSV".
- Around line 225-227: Fix the typos in the service description for the entry
with name "l2-subsetter-casper": update the description field to read "Service
that converts a NetCDF file to one or more CSV files." so "and NetCDF" becomes
"a NetCDF" and "CVS files" becomes "CSV files."
- Around line 51-75: The asdc-casper service is incomplete: update the
description to "Service that converts a NetCDF file to one or more CVS files",
bump data_operation_version from '0.18.0' to '0.22.0', and add the missing
CASPER step to the steps list by appending an entry invoking ${CASPER_IMAGE}
(e.g., a step with image: !Env ${CASPER_IMAGE} and is_sequential: true) after
the existing QUERY_CMR_IMAGE step so the service matches other -casper chains.
🧹 Nitpick comments (2)
config/services-uat.yml (2)
572-574: Consider adding a more descriptive service description.The description "HOSS-casper" is minimal compared to other services. Consider adding a description similar to
sds/HOSS-geographicbut noting the CSV output capability, e.g., "A service chain combining HOSS geographic subsetting with CASPER CSV conversion."
757-759: Consider adding a more descriptive service description.Similar to
HOSS-casper, the description "sambah-casper" is minimal. Consider referencing the full SAMBAH chain description or noting that this is the SAMBAH chain with CSV output via CASPER.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
config/services-uat.ymlpackages/util/env-defaultsservices/harmony/env-defaultsservices/harmony/test/versions.ts
🔇 Additional comments (3)
services/harmony/test/versions.ts (1)
27-48: LGTM!The test expectations correctly include all five new Casper service variants that are defined in
config/services-uat.yml. The service names match exactly and their positions in the array reflect the order defined in the YAML configuration.services/harmony/env-defaults (1)
570-574: Verify image source and consider adding CPU resource limits.Two observations:
The image is sourced from a personal GitHub account (
ghcr.io/kecunning/casper) rather than an organization likeghcr.io/nasa/orghcr.io/podaac/. Is this intentional for UAT testing, or should it be moved to an official repository before deployment?Most other backend services define
*_REQUESTS_CPUand*_LIMITS_CPUvariables (e.g.,HYBIG_REQUESTS_CPU,HYBIG_LIMITS_CPU). Consider adding these for CASPER for consistency and to ensure proper resource allocation in Kubernetes.Suggested additions for CPU resources
CASPER_IMAGE=ghcr.io/kecunning/casper:0.1.0 +CASPER_REQUESTS_CPU=128m +CASPER_LIMITS_CPU=128m CASPER_REQUESTS_MEMORY=1Gi CASPER_LIMITS_MEMORY=5Gi CASPER_INVOCATION_ARGS='./docker-entrypoint.sh'packages/util/env-defaults (1)
132-132: LGTM!The
CASPER_SERVICE_QUEUE_URLSfollows the established pattern for service queue URL definitions. The image reference and FIFO queue naming convention are consistent with other services in this file.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @config/services-uat.yml:
- Around line 572-574: The service entry for "HOSS-casper" has a too-brief
description; replace the current description value with a short paragraph that
explains what HOSS-casper does (e.g., conversions/transformations performed),
the expected input and output formats and data types, and its key capabilities
(supported collections, subsetting types, and operations). Use the existing
"HOSS-geographic" entry as a model: mention supported collections or sources,
primary processing steps, accepted input schemas and output formats, and any
notable limitations or implementation details so the description is actionable
for operators and integrators.
- Around line 757-759: Replace the minimal description for the service named
"sambah-casper" with a detailed multi-sentence description that: outlines the
full SAMBAH service chain (L2-subsetter → Batchee → STITCHEE → CONCISE →
CASPER), briefly explains the purpose of each component (e.g., L2-subsetter
filters inputs, Batchee groups/segments, STITCHEE stitches/aligns, CONCISE
compresses/summarizes, CASPER performs final processing/validation), specifies
expected input and output formats for this service (schemas, file types, or
payload structure), and lists key capabilities and supported operations (batch
sizes, transforms, validation rules, and error behaviors) so readers can
understand how "sambah-casper" fits into and interacts with the parent service
"l2-subsetter-batchee-stitchee-concise".
chris-durbin
left a comment
chris-durbin
left a comment
There was a problem hiding this comment.
It's probably worth having a discussion on whether we want to just add a conditional CASPER step to existing chains instead of creating several new chains.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@config/services-uat.yml`:
- Around line 612-615: Update the ASDC CASPER service description in
config/services-uat.yml to correct two typos: change "outputing" to "outputting"
in the sentence describing conversion to CSV, and change "Outputs csv" to
"Outputs CSV" to match capitalization/consistency; locate and edit the ASDC
CASPER service block where those phrases appear.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
config/services-uat.ymlservices/harmony/test/versions.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- services/harmony/test/versions.ts
🔇 Additional comments (4)
config/services-uat.yml (4)
51-78: LGTM! Previous review issues have been addressed.The service now correctly includes the CASPER step, uses the updated
data_operation_version: '0.22.0', and the description typos have been fixed.Minor observation: The conditional
format: ['text/csv']on the CASPER step (lines 76-77) is technically redundant sincetext/csvis the only supported output format. This is harmless but could be simplified.
227-227: No concerns with whitespace changes.
663-666: Verify ifalways_wait_for_prior_stepis needed for the CASPER step.The previous review raised a question about whether CASPER is an aggregation service. If CASPER is intended to convert the final concatenated output from CONCISE to CSV, it may need
always_wait_for_prior_step: trueto ensure it receives the fully concatenated file rather than intermediate outputs.If CASPER operates on individual files independently (not aggregating), then the current configuration without
always_wait_for_prior_stepis correct.Could you clarify CASPER's processing model? If it should only process the final concatenated output when concatenation is requested, consider:
- image: !Env ${CASPER_IMAGE} + always_wait_for_prior_step: true conditional: format: ['text/csv']
637-637: LGTM! Output format addition enables CASPER chain selection.Adding
text/csvtooutput_formatscorrectly advertises this capability and allows users to trigger the CASPER step by requesting CSV output.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…ult. In the SAMBAH chain do not call concise first before casper - only call concise for netcdf4 output requests and casper for csv outputs.
chris-durbin
left a comment
chris-durbin
left a comment
There was a problem hiding this comment.
Tested successfully in a harmony sandbox environment.
… format is requested.
5 participants
Jira Issue ID
TRADE-738
Description
These are the updates for deploying CASPER service in UAT Harmony environment. Included are the addition of the CASPER services and 4 CASPER service chains: sambah-casper, hoss-casper, l2-subsetter-casper, l2-subsetter-consice-casper. The chains are detailed in the attached word [doc.](CASPER chain Descriptions.docx)
Local Test Steps
Submit harmony requests for sambah, hoss, l2-subsetter and l2-subsetter-consice, and add "format = 'text/csv'" as a parameter in the request like:
request = Request(
collection=collection,
spatial = BBox(-114.42214, 29.72474,-62.2058, 40.6032),
max_results = 2,
extend = "mirror_step",
concatenate = True,
format = 'text/csv',
)
The addition of format = 'text/csv' selects a CASPER chain.
PR Acceptance Checklist
Summary by CodeRabbit
New Features
Documentation
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.