Skip to content

DAS-2446 - Allow HOSS to return an unexecuted OPeNDAP URL when requested.#61

Merged
lyonthefrog merged 19 commits intomainfrom
DAS-2446
Dec 17, 2025
Merged

DAS-2446 - Allow HOSS to return an unexecuted OPeNDAP URL when requested.#61
lyonthefrog merged 19 commits intomainfrom
DAS-2446

Conversation

@lyonthefrog
Copy link
Copy Markdown
Contributor

@lyonthefrog lyonthefrog commented Dec 11, 2025

Description

This PR allows HOSS to accept a new format parameter application/x-netcdf4;profile=opendap_url, which will trigger HOSS to return an unexecuted OPeNDAP URL instead of a subset file. The URL format matches output currently used in the GES DISC GOSS application. A few variable names were updated to not assume that we're working with a subset output file.

This will involve a new Harmony service chain (currently called sds/hoss-url-l3), but it's not yet solidified. We'll likely change the name, and add the new format option to the service record once CMR and EDSC folks make the required changes. The new format parameter needs to be added to the relevant services in Harmony's services-uat.yml, so there currently exists an in-progress Harmony branch DAS-2444 for the following tests to be run through.

Jira Issue ID

DAS-2446

Local Test Steps

Open Docker Desktop and pull this branch. Also pull and checkout the DAS-2444 harmony branch.

Run HOSS unit tests:

./bin/build-image && ./bin/build-test && ./bin/run-test 

Bootstrap Harmony (remember to be in the DAS-2444 branch):

./bin/bootstrap-harmony

Notebooks

In http://localhost:3000:

Run notebook in ticket (HOSS_output_URLs.ipynb):

  • Environment dependencies: harmony-py
  • Follow verification instructions in ticket

Run ticket/regression tests:

  • nsidc-smap_Regression.ipynb (harmony-regression-tests)
  • HOSS_Regression.ipynb (harmony-regression-tests)

PR Acceptance Checklist

  • Jira ticket acceptance criteria met.
  • CHANGELOG.md updated to include high level summary of PR changes.
  • docker/service_version.txt updated if publishing a release.
  • Tests added/updated and passing.
  • [NA] Documentation updated (if needed).
  • Jira ticket updated with expected fixversion hoss-X.Y.Z

@sudha-murthy sudha-murthy self-assigned this Dec 11, 2025
Comment thread hoss/utilities.py
Comment thread hoss/adapter.py
Copy link
Copy Markdown
Contributor

@sudha-murthy sudha-murthy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR looks great,
Have a few questions and comments.

Comment thread CHANGELOG.md
Comment thread hoss/utilities.py
Comment thread hoss/subset.py
Comment thread hoss/subset.py
Comment thread hoss/utilities.py
Comment thread tests/test_adapter.py
Comment thread tests/test_adapter.py
Comment thread tests/unit/test_subset.py Outdated
@sudha-murthy
Copy link
Copy Markdown
Contributor

sudha-murthy commented Dec 12, 2025

The tests in the notebook all succeeded after running harmony from DAS-2444 branch and running reload-services-config after starting up harmony. The regression tests succeeded as well.

Comment thread tests/test_adapter.py
Comment on lines +102 to +124
actual_catalog = items[0].assets['data'].to_dict()
expected_catalog = {
'href': expected_href,
'title': expected_title,
'type': expected_mimetype,
'roles': ['data'],
}

# Check all the dictionary values match except for the href value.
self.assertTrue(
all(
actual_catalog[key] == expected_catalog[key]
for key in actual_catalog
if key != 'href'
)
)

# The href value must be compared separately because it can contain
# an constraint expression in the case where an unexecuted OPeNDAP URL
# is requested, where the variable order is not consistent.
self.assertEqual(
sorted(actual_catalog['href']), sorted(expected_catalog['href'])
)
Copy link
Copy Markdown
Contributor Author

@lyonthefrog lyonthefrog Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All this code was added to replace the simple dictionary comparison because now it's possible for href to contain the OPeNDAP URL, which contains the constraint expression where the variable order is inconsistent since required_variables is a set. There may be a less wordy way of doing this though, so open to suggestions!

Comment thread hoss/utilities.py
Copy link
Copy Markdown
Contributor

@sudha-murthy sudha-murthy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The harmony-regression-tests ran fine. The nsidc soap regression tests may need to be -re-run

@lyonthefrog lyonthefrog reopened this Dec 15, 2025
Copy link
Copy Markdown
Member

@flamingbear flamingbear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks pretty good so far. I have a couple of questions and some suggestions and a few minor things to clean up I think. Let me know if I'm not clear anywhere.

Comment thread hoss/adapter.py
Comment thread hoss/utilities.py
Comment thread hoss/utilities.py Outdated
Comment thread hoss/utilities.py Outdated
Comment thread tests/unit/test_adapter.py Outdated
Comment thread tests/unit/test_adapter.py Outdated
Comment thread CHANGELOG.md Outdated
@flamingbear
Copy link
Copy Markdown
Member

You might consider updating the hoss regression tests with one that asks for an unexecuted URL, then executes it in the notebook just like a full request.

Copy link
Copy Markdown
Member

@flamingbear flamingbear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing all of the feedback. I think now you just have snyk issues to handle. I believe they updated snyk's python version today. There may be cached values somewhere. You are getting both an unsolvable error and a failed to parse the manifest file, which has been a problem in a few repos when trying to address urllib3 vulnerabilities. But this PR has 0 changes to the dependencies. So You can close and reopen and hope and if that fails I would try to fix in a new PR.

@lyonthefrog lyonthefrog reopened this Dec 16, 2025
@lyonthefrog lyonthefrog reopened this Dec 17, 2025
@lyonthefrog lyonthefrog mentioned this pull request Dec 17, 2025
6 tasks
@lyonthefrog lyonthefrog merged commit 12b7b40 into main Dec 17, 2025
4 checks passed
@lyonthefrog lyonthefrog deleted the DAS-2446 branch December 17, 2025 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants