Skip to content
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

Feature/issue 13 - Add SWORD version from shp.xml #111

Merged
merged 7 commits into from
Mar 18, 2024

Conversation

torimcd
Copy link
Collaborator

@torimcd torimcd commented Mar 7, 2024

Github Issue: #13

Description

Add the SWORD version number to each database record

Overview of work done

Added new parse_metadata_from_shpxml function to swot_reach_node_shp io handler to read metadata from the *.shp.xml files.

Overview of verification done

Updated test_query unit test to explicitly check new sword_version value is present and correct. Added sword_version field to test values in constants file to ensure it is covered in both the read_shapefile tests and the test_hydrocron_database tests (ie test that it is read correctly from the file, and that it is loaded correctly into the db). All unit tests pass.

Overview of integration done

Deployed to SIT and verified newly loaded granules have new sword_version attribute with correct values.

PR checklist:

  • Linted
  • Updated unit tests
  • Updated changelog
  • Integration testing

See Pull Request Review Checklist for pointers on reviewing this pull request

Copy link
Collaborator

@nikki-t nikki-t left a comment

Choose a reason for hiding this comment

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

Modifications look good to me. I will need to update the timeseries API endpoint to return the SWORD version. Should we always return the SWORD version in the response or should we make it an optional field?

@frankinspace frankinspace merged commit 62e5bc6 into develop Mar 18, 2024
@frankinspace frankinspace deleted the feature/issue-13 branch March 18, 2024 17:20
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