Closed cchadowitz closed 1 year ago
Hello @cchadowitz-pf I think putting labels in the default call is fine, the label list in not always this long, and even if it is, it's not that big of a problem.
I have an ongoing PR to change this part of the code to use oatpp::DTO (#1506) so this PR will become invalid. I can add the labels afterwards, and add a unit test.
This pull request is now in conflict :(
Hi @Bycob - no problem at all, thanks!
After thinking about this a bit more, I'd actually probably prefer a separate call if that's no more complicated (when you do get around to this). I realized we currently use the default call as a health check, and dumping potentially a couple thousand labels into the output would add a decent amount of unnecessary data to it. Our use case for returning the available labels is just to validate a few things once at startup, rather than a regular check-in.
So, if it's not more complicated to do so, I'd actually rather a separate call for the labels :) But if that's not straightforward, we can work with this as well. Thanks!
We could also include labels in the info
call conditioned by a query variable, e.g. curl -X GET http://localhost:8080/services/placeshybrid?labels=true | jq
That would also be great! 👍
Implemented by #1508
This adds the full list of labels from the corresp.txt file (if present) for the service model when the
/services/<name>
endpoint is hit. E.g.produces the following with this change:
For some models (like this one), it can result in quite a long output, so I'm open to the possibility of adding a new separate endpoint (perhaps
/services/<name>/labels
or/services/<name>/categories
) but that was a bigger code change overall. The way it is in this change is easy to pull out programmatically with something likejq
:Currently the categories/labels are returned in no particular order, just from iterating through the
unordered_map
they were stored in originally.