Closed rma-rripken closed 2 years ago
Adam provided the following link to the data object package https://bitbucket.hecdev.net/projects/SC/repos/hec-monolith/browse/hec-monolith/src/main/java/hec/data/level
These changes (or something similar) is needed by the CWMSVue Location Levels tab.
Here is a screenshot of the level tab in CWMSVue. The tree is broken down from location level ids mapped to their effective dates (which includes constant/seasonal value data).
One option is for Radar to parse the json output from the pl/sql and turn it into DTO objects. Another option is to not go thru the pl/sql and to reuse either https://bitbucket.hecdev.net/projects/CWMS/repos/hec-cwms-data-access/browse/cwms-db-jooq/src/main/java/usace/cwms/db/jooq/dao/CwmsDbLevelJooq.java#33 or https://bitbucket.hecdev.net/projects/CWMS/repos/hec-cwms-data-access/browse/hec-db-jdbc/src/main/java/wcds/dbi/oracle/cwms/CwmsLevelJdbcDao.java#302
Adam pointed out that the level tab is already slow. What can we do to make it not slow?
skip the pl/sql. Let's generate in Java. we can paginate and let the tab update overtime as well as continue to tweak the query performance. (I can handle the performance part if you can just get it working.)
I would have the primary query return the locations, the the times. similar to how the extents work; maybe simple metadata (like if single value). CWMSVue could then query the database for more information when the user picks it.
I mean that's have the trick to fast UI, don't load it unless you have to, and cache what you do load as long as possible (which we really need to set better in RADAR. most of this doesn't change every second. 5-10 minutes would let a few refreshes be faster... just a though.)
The HEC DB Levels API and catalog call is used by many different applications, clients, tools, etc. The catalog call's API returns the data as List
Regardless of whether we to split this call into two separate calls, this call is required to be backward compatible. Without a catalog of the full data, I would need to eagerly call the getOne endpoint for every returned level id which wouldn't be performant even in parallel.
I'm following that impl very closely but with paging.
@adamkorynta does it need just to the effective date? or to includes the contents that would be under the effective date? e.g. would
"name": "The Dam",
... (other info),
"effective-dates": [
"then",
"now",
"soon"
]
]```
work or does it need to be:
```[
...,
"values": [
{ "date-time": "then", "value": 5 },
...
]
]```
which could get a little tricky with time-series and such. I support the former for sure.
If we *must* do the latter it needs to be hidden behind a parameter and should be non-default.
See https://github.com/USACE/cwms-radar-api/wiki/Design-Goals, the 2nd under "not a high maintenance burden", but it shouldn't be the default behavior.
(On the other hand I think you're asking for the 1st option it just wasn't super clear to me.)
It's the second case since the interface for the DAO returns the values. Hiding behind a parameter like the "include-assigned" for location groups works just fine for me. Having an additional endpoint for a levels/data-catalog or some such would also work.
Since there is no getOne method currently, I can't compare the performance impact of trying to retrieve all of the level data through parallel getOne calls.
I would think the performance would be comparable but the GUI possibilities can make it seem faster.
e.g.
At least that's likely how I'd right a web based version from scratch.
On the other hand, as far as getting the data out of the database itself all at once could be faster except maybe in the case of a timeseries based level. (Ignoring the possibility of cache, a level doesn't actually change often so lends itself to cache.
... really need to figure out how to get memcached or redis involved in this long term.... anyways, I digress.
The API is used beyond CWMSVue, I think my screenshot might have been distracting. I was trying to show that the API call isn't just the level ID's without having to dig into the source code.
The timeseries based level just returns a timeseries id rather than the values since there is no time window parameter. There is a separate API call for resolving a level to a time series, which isn't scoped for the current work. It really is just all the rows from cwms_v_location_level, but with optional filters on location id and office id.
oh, I've just been misinterpreting the whole time. yeah that volume of data should be fine to just do directly without special handling.
You're right it's not really practical to operate with out all the effective date meta data and except for time series it's all pretty small. (well I guess some seasons could get big.)
(If you were trying to list the BLOB table in CWMSVue though, that would be a different story.)
The Levels controller getAll returns raw json straight from pl/sql. Its also missing a description of the content type in the responses section of the annotation.
I sort of understand why the controller didn't provide the content type - its just passing thru the pl/sql output. But not providing that information makes it more difficult for clients - they can't use codegen to map the Radar output to the language objects of their choice.
We should make the LevelController return specific DTO objects when given json2 accept header.
LevelController should also have a getOne method.
We should also consider adding catalog like methods to get a list of the effective dates for a specified level at a location.