Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REST] Optionally don't include channels when listing things #3324

Closed
ccutrer opened this issue Jan 17, 2023 · 9 comments
Closed

[REST] Optionally don't include channels when listing things #3324

ccutrer opened this issue Jan 17, 2023 · 9 comments

Comments

@ccutrer
Copy link
Contributor

ccutrer commented Jan 17, 2023

Inspired by #3300, I noticed that my /rest/things endpoint is 2.6MB, because it includes every channel for every thing. For things like the Things list in MainUI, you don't need the channels, so it would be nice if the API allowed optionally excluding channels.

@J-N-K
Copy link
Member

J-N-K commented Jan 18, 2023

I don't think we should do that. Channels are integral parts of the thing and should therefore be reported. It would be interesting to see how much compression can be achieved by openhab/openhab-distro#1460. You can just add the seven lines to the jetty.xml (and probably restart).

@spacemanspiff2007
Copy link
Contributor

spacemanspiff2007 commented Jan 18, 2023

I'd like to see that because I often want to check things but am not interested in the channels.
On the items we have the fields parameter which allows to specify fields that will be sent back.
I think that would work nicely on the thing endpoint, even though I'd rather like to specify the fields that will not be sent back.

@J-N-K
Copy link
Member

J-N-K commented Jan 18, 2023

@spacemanspiff2007 In #3298 (comment) you argue that fetching namespace data only (which is a some bytes only) instead of the full item (which is more on the order of kB) is wrong, because JSON processing is fast and the larger amount of data doesn't matter. Here you argue exactly the opposite. What is the difference?

@spacemanspiff2007
Copy link
Contributor

It's not a contradiction, even if it looks like one. 😉
I still think creating an additional endpoint to request a subset of the same data structure is not a good idea.

This is not for performance reasons but rather a tooling issue.
Currently it's not possible to collapse jsons in the ApiExplorer (swagger) so it's very hard to find something in the response of the things endpoint.
Currently I first request the /things with summary=true and then get the uuid and request the thing from the endpoint and scroll down because the channels are at the top.

Maybe it makes sense to implement the response reducer in a generic way so it can be simply reused for the other get endpoints (/things, /channels, etc).

But tbh. I have no strong opinion on this one because I can always open a 3rd party tool and load the complete response, there.

PS:
On the items and items/<item> endpoint it's already possible to reduce the fields so one can request e.g.

{
  "metadata": {
    "namespace": {"value": "asdf"}
  }
}

@ccutrer
Copy link
Contributor Author

ccutrer commented Jan 18, 2023

Ohhh, I now see there's a summary param, which does essentially what I want. I think I can close this ticket. Though maybe that can be better documented somewhere?

Regarding the more general discussions we've been having about including fields or not, many modern web apps are moving to GraphQL instead of REST, and the client is required to provide a query naming precisely which fields they want. This not only helps responsiveness, but makes it easy to get the minimal bytes across the wire for bandwidth limited scenarios like mobile apps. It's also nice that you're not having to discuss with each individual API which fields to include by default, and which to exclude, etc, etc. But that'd be a pretty massive change, has disadvantages (not as easy to use from all the scripts and integrations users write to interact with openHAB), and probably not even worth discussing in the 4.0 timeframe.

@ccutrer ccutrer closed this as completed Jan 18, 2023
@J-N-K
Copy link
Member

J-N-K commented Jan 18, 2023

But what would be the benefit of having more information than we get with "summary=true" but without channels? I fail to the the use-case for that.

@spacemanspiff2007
Copy link
Contributor

But what would be the benefit of having more information than we get with "summary=true" but without channels? I fail to the the use-case for that.

There is none.
If there would be a generic reducer one could have dropped the summary parameter.
I often check the thing properties and configuration which of course are missing in the summary.
But that's not worth modifying the current implementation

GraphQL

GraphQL has some pros but lots of other issues (e.g. it's very hard for newcomers, no swagger, playground is hard) and for local application bandwidth just doesn't matter. But that's stuff for another issue 😉

@ccutrer
Copy link
Contributor Author

ccutrer commented Jan 22, 2023

A front-end guy has chased down one of the extra full calls in MainUI to /rest/things (openhab/openhab-webui#1650) and discovered it's to populate a bridge picker. @J-N-K: what are your thoughts on adding a binding= and/or a onlyBridges param?

@ghys
Copy link
Member

ghys commented Jan 22, 2023

It's not necessary IMHO - /rest/things?summary=true already excludes the channels.
(this particular front-end guy should know, he added the option; #1827 😄)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants