Add hoss-opendap-url output option to VOSS config in production.#886
Add hoss-opendap-url output option to VOSS config in production.#886chris-durbin merged 2 commits intomainfrom
Conversation
📝 WalkthroughWalkthroughTwo YAML service configuration files were updated to expand output format support and clarify service descriptions. The production config adds Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
config/services-prod.yml (1)
235-243:⚠️ Potential issue | 🔴 CriticalCritical: Missing 'reformat' operation for OPeNDAP URL format.
The production configuration adds
application/x-netcdf4;profile=opendap_urlto the supported output formats, but theoperationslist at line 243 only includes['variableSubset']. The UAT configuration (line 353) correctly includes['variableSubset','reformat'].Since
application/x-netcdf4;profile=opendap_urlis a reformatting operation, the HOSS image step must declare 'reformat' in its operations list. Without this, Harmony won't route reformatting requests to this service, and the fix for the error mentioned in the PR description will not work.🔧 Proposed fix
- image: !Env ${HOSS_IMAGE} - operations: ['variableSubset'] + operations: ['variableSubset','reformat']🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/services-prod.yml` around lines 235 - 243, The HOSS step in the production config declares output_formats including "application/x-netcdf4;profile=opendap_url" but its steps block (the HOSS image entry with operations=['variableSubset']) is missing the 'reformat' operation; update the HOSS image step's operations list to include 'reformat' (i.e., operations: ['variableSubset','reformat']) so requests for the OPeNDAP URL profile will be routed to the HOSS service for reformatting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@config/services-prod.yml`:
- Around line 235-243: The HOSS step in the production config declares
output_formats including "application/x-netcdf4;profile=opendap_url" but its
steps block (the HOSS image entry with operations=['variableSubset']) is missing
the 'reformat' operation; update the HOSS image step's operations list to
include 'reformat' (i.e., operations: ['variableSubset','reformat']) so requests
for the OPeNDAP URL profile will be routed to the HOSS service for reformatting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3748d844-3e9e-4286-b490-828900a5d9e0
📒 Files selected for processing (2)
config/services-prod.ymlconfig/services-uat.yml
chris-durbin
left a comment
There was a problem hiding this comment.
The change looks fine - once I merge in the build fix I'll update this branch and then the tests should pass.
Jira Issue ID
DAS-2448
Description
We forgot to add the hoss-opendap-url option to the VOSS configuration in Production
Local Test Steps
Does anyone know how to test that this notebook will work in production, or can you eyeball it and know whether or not it the appropriate changes have been made?
hoss-2445-final-notebook.zip
Previously it gave the following error that should now produce valid happy output:
PR Acceptance Checklist