USACE / cwms-data-api

Corps Water Management System RESTful Data Service
MIT License
13 stars 14 forks source link

Add parameters to timeseries post/patch endpoints #266

Closed adamkorynta closed 1 year ago

adamkorynta commented 2 years ago

The CWMS client makes use of various store rules and override protection boolean flags for its various processes. Additionally, the createAsLrts flag for identifying pseudo regular time series intervals vs local regular intervals is also needed.

Currently the defaults are store rule=REPLACE_ALL , override protection=false, and createAsLrts=false https://github.com/USACE/cwms-radar-api/blob/develop/cwms_radar_api/src/main/java/cwms/radar/data/dao/TimeSeriesDaoImpl.java#L887

Mike Neilson brought up that special permissions should be investigated for handling destructive store rules such as DELETE_INSERT, especially on national instance.

adamkorynta commented 1 year ago

Needs to also support storing the time series version

MikeNeilson commented 1 year ago

@DanielTOsborne think you could take on these changes? Inside the TimeSeriesDAOImpl is already placeholders.

DanielTOsborne commented 1 year ago

Not sure about the "special permissions" part, but the rest shouldn't be an issue. I'll take a look at this.

rma-psmorris commented 1 year ago

Worked with Adam to re-prioritize issues that are blocking for https://github.com/HydrologicEngineeringCenter/cwms-radar-client

adamkorynta commented 1 year ago

Worked with Peter and Ryan to identify blocking issues and priorities for https://github.com/HydrologicEngineeringCenter/cwms-radar-client

rma-rripken commented 1 year ago

Worked with Peter and Adam to identify blocking issues and priorities for https://github.com/HydrologicEngineeringCenter/cwms-radar-client

adamkorynta commented 1 year ago

Swagger docs don't specify what the store-rule or override-protection parameters behavior is if not present.

The version-data says that a "null" version will be used, but it should probably elaborate on what that means.

adamkorynta commented 1 year ago

The override protection and create as LRTS are defined as Strings instead of booleans

adamkorynta commented 1 year ago

I am unable to get the version date store to work. The only guess that I have is this line: https://github.com/USACE/cwms-radar-api/blob/develop/cwms_radar_api/src/main/java/cwms/radar/data/dao/TimeSeriesDaoImpl.java#L835 where versioned boolean is always false.

MikeNeilson commented 1 year ago

That is not yet implemented. It's on the TODO list. actually some sample data or even a test with appropriate data to target would make that faster.

MikeNeilson commented 1 year ago

then again that might just be as easy as allow the parameter, for write anyways

adamkorynta commented 1 year ago

I'm not sure what you mean by not yet implemented, the version date is stored in the ts store routine here: https://github.com/USACE/cwms-radar-api/blob/develop/cwms_radar_api/src/main/java/cwms/radar/data/dao/TimeSeriesDaoImpl.java#L879 and the query parameter exists.

MikeNeilson commented 1 year ago

It's staged for implementation. we have no tests and there's no GET support at this time.

MikeNeilson commented 1 year ago

though super happy if I'm wrong.

adamkorynta commented 1 year ago

Yeah I guess the GET isn't there, but the catalog should be returning versions just fine. The POST code looks like it should work just fine. I don't think the store ts data should even be calling the create ts code, but maybe that's a different discussion.

MikeNeilson commented 1 year ago

I agree on createTsCode, but instead of arguing too much I just "fixed" it in the last merge. I'll will open a new issue for that topic.

adamkorynta commented 1 year ago

The other way to tackle it is from the schema side, if we are sending in a version date in store_ts, then it should switch the versioned boolean from false to true.

adamkorynta commented 1 year ago

All my client tests are now passing for this feature. I think it is good to close.