Repository navigation
Conversation
RyanM-RMA
changed the title
| @@ -117,7 +118,7 @@ protected WaterSupplyAccountingDao getWaterSupplyAccountingDao(DSLContext dsl) { | |||
| @OpenApiParam(name = ASCENDING, description = "Whether or not the entries should be returned " | |||
| + "in ascending order. Defaults to TRUE.", type = Boolean.class), | |||
| @OpenApiParam(name = ROW_LIMIT, description = "The maximum number of rows to return. " | |||
There was a problem hiding this comment.
Do we want to use -1 for "no limit" ?
There was a problem hiding this comment.
That's what's used for time series.
There was a problem hiding this comment.
We were modelling this behavior off the way turbines and gate changes work. Negative row limit is used to say how many rows before the end date, vs positive row limit where it's how many rows after the start date.
There was a problem hiding this comment.
Gates, turbines, and water supply treat the row limit as follows:
- negative values limit rows off of head
- positive values limit rows off of tail
- to not have a row limit, Integer.MAX_VALUE is specified (yes, this is an annoyingly large value).
There was a problem hiding this comment.
@MikeNeilson @rma-bryson -- do you have any additional feedback on this PR or can it be approved?
There was a problem hiding this comment.
If we want to keep the head/tail behavior, I suggest we create additional parameters.
|
@rma-psmorris - you were going to follow up on this PR with a question in regards to granularity of water supply accounting retrieval being to the pump or contract |
Retrieval granularity is to the project, water user, contract. Pump is not included, so accounting retrieved would be all pumps associated with this contract (pump in, pump out, pump out below). |
c61144a to
b6c9723
Compare
MikeNeilson
left a comment
MikeNeilson
left a comment
There was a problem hiding this comment.
Generally looks reasonable. I just forced a rebase since I merged in #1985, will likely need some minor corrections to imports.
| @@ -117,7 +118,7 @@ protected WaterSupplyAccountingDao getWaterSupplyAccountingDao(DSLContext dsl) { | |||
| @OpenApiParam(name = ASCENDING, description = "Whether or not the entries should be returned " | |||
| + "in ascending order. Defaults to TRUE.", type = Boolean.class), | |||
| @OpenApiParam(name = ROW_LIMIT, description = "The maximum number of rows to return. " | |||
There was a problem hiding this comment.
If we want to keep the head/tail behavior, I suggest we create additional parameters.
What additional parameters are you thinking? Also, given that this is the only parameter in the other endpoints, how would we adjust those endpoints to be consistent? Would those endpoints need to be backwards compatible as well? Also, I'm not sure how to reply to the other comment about creating additional parameters...it doesn't let me quote reply. |
Summary
Adding check for row limit, changing default to 500, and updating documentation for default row limit of 500.
Related Issue
Closes #1920
Jira Ticket: https://jira.hecdev.net/browse/CDA-140
Validation
I'll be adding another change for integration tests on row limits.
Checklist