USACE / cwms-data-api

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

Add capability to delete time series time window #303

Closed adamkorynta closed 1 year ago

adamkorynta commented 1 year ago

Currently the CWMS client uses the following parameters:

rma-rripken commented 1 year ago

Do you envision those as optional queryParameters to the existing timeseries controller delete?

MikeNeilson commented 1 year ago

That seems reasonable to me.

adamkorynta commented 1 year ago

The "start-time" and "end-time" parameter names are inconsistent with the "begin" and "end" parameters in the GET request.

adamkorynta commented 1 year ago

'ts_item_mask' doesn't follow the dash delimiter style. It is also called a "cookie" in the docs and I'm not sure what that means. Should probably avoid that term for confusion with HTTP cookie.

adamkorynta commented 1 year ago

I'm actually confused what is the ts_item_mask ? The time series id is already a required endpoint parameter, so I'm not sure what a ts mask would do. Oh, and it is an integer.

adamkorynta commented 1 year ago

The start-time/end-time docs say " or default time zone" and doesn't have the formatting examples like the other time fields. The default time zone will use the zone in the ISO 8601 string? or is default something else?

adamkorynta commented 1 year ago

Override protection docs don't specify what the default behavior is if not provided. Personally, I'd rather not have an "E" option and just let the default behavior error when deleting protected values.

rma-rripken commented 1 year ago

All great points. #325

adamkorynta commented 1 year ago

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