add forecast client API for python #35
Conversation
MikeNeilson
left a comment
MikeNeilson
left a comment
There was a problem hiding this comment.
Looks good to me. Have a few questions. But I'll let the people who are more directly working within the cwms python approve or not.
| "issue-date": issue_date.isoformat(), | ||
| } | ||
|
|
||
| headers = {"Accept": constants.HEADER_JSON_V2} |
There was a problem hiding this comment.
I didn't notice this in the other work but this is the first version of this data, so we can just use plain application/json. the V2 json was to provide a difference for the original from the DB outputs.
There was a problem hiding this comment.
Hmm I thought the pattern was all v1 was direct to db and starting v2 was not, even for new code. All of the new data types (binary, text, etc) are using V2.
There was a problem hiding this comment.
No it was "first instance of type" is V1.
... whelp, apparently I wasn't paying attention as well as I thought (to be fair I was focusing on the logic).
But it's not a huge deal, we'll be making the application/json be a aliases to the prefered anyways.
cwms/forecast/forecast_instance.py
Outdated
| params = { | ||
| constants.OFFICE_PARAM: office, | ||
| constants.NAME: spec_id, | ||
| "designator": designator, |
There was a problem hiding this comment.
Shouldn't this be a constant as well?
There was a problem hiding this comment.
yeah, forgot to do that
| "first-date-time": 1692702150000, | ||
| "last-date-time": 1727017260000, | ||
| "max-age": 5, | ||
| "time-series-count": 3, |
There was a problem hiding this comment.
are we planning a separate endpoint to get the time series or time series id's. I wouldn't expected a list somewhere like the location-ids above.
There was a problem hiding this comment.
The timeseries id's come back as a list for the spec. No need for a separate endpoint for the forecast timeseries since we already get the list of ids.
There was a problem hiding this comment.
Duh, I can read. This is what happens when you review things 30 minutes after actually waking up.
3 participants
adds spec and instance endpoints with unit test coverage via mock server