Skip to content

CDA-113 Adding csv support for timeseries#1780

Merged
rma-bryson merged 6 commits into
developfrom
feature/CDA-113-support-csv-for-time-series
Jun 25, 2026
Merged

CDA-113 Adding csv support for timeseries#1780
rma-bryson merged 6 commits into
developfrom
feature/CDA-113-support-csv-for-time-series

Conversation

@rma-bryson

@rma-bryson rma-bryson commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

Summary

Implements csv support for timeseries. This should also streamline other data types supporting our custom csv formatting in the future - including metadata comment headers at top of payload, unit in column header, and optional-column inclusion via DTO annotations in the spirit of Jackson.

This also adds support for specifying date-format. Currently this only works for csv, but could be extended to other formats in the future.

Also noting decision during cda dev meeting to use query parameters for optional column inclusion and date-format. Updated ADR to reflect that.

Related Issue

Closes #1257

Validation

Added unit tests and integration tests

Checklist

  • Junie assisted in getting annotations working correctly.

@rma-bryson rma-bryson force-pushed the feature/CDA-113-support-csv-for-time-series branch 10 times, most recently from 7219b05 to 8465b44 Compare June 18, 2026 16:15
@rma-bryson rma-bryson marked this pull request as ready for review June 18, 2026 16:18
@rma-bryson rma-bryson requested a review from MikeNeilson June 18, 2026 16:18
@rma-bryson rma-bryson force-pushed the feature/CDA-113-support-csv-for-time-series branch from 8465b44 to 76e0901 Compare June 18, 2026 16:26

@MikeNeilson MikeNeilson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Generally good. Definitely concerned about some critical logic duplication.

Comment thread cwms-data-api/src/main/java/cwms/cda/api/Controllers.java Outdated
Comment thread cwms-data-api/src/main/java/cwms/cda/data/dao/StreamConsumer.java
Comment thread cwms-data-api/src/main/java/cwms/cda/data/dao/TimeSeriesDaoImpl.java Outdated
Comment thread cwms-data-api/src/main/java/cwms/cda/data/dao/TimeSeriesDaoImpl.java Outdated
@rma-bryson rma-bryson requested a review from MikeNeilson June 18, 2026 19:58
@rma-bryson

Copy link
Copy Markdown
Collaborator Author
image Was able to get the example csv working

MikeNeilson
MikeNeilson previously approved these changes Jun 22, 2026

@MikeNeilson MikeNeilson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice trick with the getExample method to fill out that detail.

Seems within that we could also setup our own annotation to retrieve a sample JSON.

Comment thread cwms-data-api/src/main/java/cwms/cda/data/dao/StreamConsumer.java
Comment thread cwms-data-api/src/main/java/cwms/cda/ApiServlet.java Outdated
Comment thread cwms-data-api/src/main/java/cwms/cda/data/dao/TimeSeriesDaoImpl.java Outdated
@rma-bryson rma-bryson force-pushed the feature/CDA-113-support-csv-for-time-series branch from 9f402f6 to ab100dc Compare June 23, 2026 19:00
Comment thread cwms-data-api/src/main/java/cwms/cda/data/dao/TimeSeriesDaoImpl.java Outdated
@rma-bryson rma-bryson requested a review from MikeNeilson June 23, 2026 20:30
MikeNeilson
MikeNeilson previously approved these changes Jun 24, 2026

@MikeNeilson MikeNeilson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry, apparently I merged something in that caused a conflict in this PR.

@rma-bryson rma-bryson force-pushed the feature/CDA-113-support-csv-for-time-series branch 2 times, most recently from eaa3133 to d55690a Compare June 24, 2026 20:17
@rma-bryson rma-bryson requested a review from MikeNeilson June 24, 2026 20:19
@rma-bryson

Copy link
Copy Markdown
Collaborator Author

Sorry, apparently I merged something in that caused a conflict in this PR.

Rebased, ready to merge once approved

…Moved CsvOnDemandInputStream into its own file. Removed redundant unit conversion logic. Added csv example generator.
…handle invalid requested units gracefully."

This reverts commit 9f402f6.
…re the data query, eliminating any per-row conversion overhead while still failing fast on invalid units.
@rma-bryson rma-bryson force-pushed the feature/CDA-113-support-csv-for-time-series branch from d55690a to 892b6a0 Compare June 24, 2026 21:41
@rma-bryson rma-bryson merged commit dac29e2 into develop Jun 25, 2026
8 checks passed
@rma-bryson rma-bryson deleted the feature/CDA-113-support-csv-for-time-series branch June 25, 2026 16:27
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.

Changing format= Does Not Work with Request Headers

2 participants