-
-
Notifications
You must be signed in to change notification settings - Fork 430
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] New REST APIs to parse/generate syntax for items and things #4569
base: main
Are you sure you want to change the base?
Conversation
521b446
to
2ce9669
Compare
Here is an example of result for things:
and the same in flat mode:
|
And the result for items:
|
cff0a29
to
c3d70dc
Compare
@kaikreuzer and @openhab/core-maintainers : for your information, the PR is now ready for review. |
Go back to draft as I discovered a bug. |
c3d70dc
to
6bc963f
Compare
Problem with items sorting is solved. So ready again. |
6bc963f
to
16bcb14
Compare
Related to openhab#4509 Added REST APIs: - GET /inbox/{thingUID}/filesyntax to generate file syntax for the thing associated to the discovery result - GET /items/filesyntax to generate file syntax for all items - GET /items/{itemname}/filesyntax to generate file syntax for an item - GET /things/filesyntax to generate file syntax for all things - GET /things/{thingUID}/filesyntax to generate file syntax for a thing All these APIs have a parameter named "format" to request a particular output format. Of course, a syntax generator should be available for this format. Only "DSL" format is provided by this PR as this is currently our unique supported format for items and things in config files. So this parameter is set to "DSL" by default. In the future, new formats could be added and they will be automatically supported by these APIs. The API GET /things/filesyntax has another parameter named "preferPresentationAsTree" allowing to choose between a flat display or a display as a tree. Its default value is true for a display of things as tree. Signed-off-by: Laurent Garnier <[email protected]>
16bcb14
to
3c2f376
Compare
Signed-off-by: Laurent Garnier <[email protected]>
Signed-off-by: Laurent Garnier <[email protected]>
52ce3b9
to
506527b
Compare
Signed-off-by: Laurent Garnier <[email protected]>
Each API has a parameter named "hideDefaultParameters" to hide or show the configuration parameters having the default value. They are hidden by default. Signed-off-by: Laurent Garnier <[email protected]>
Signed-off-by: Laurent Garnier <[email protected]>
Signed-off-by: Laurent Garnier <[email protected]>
aff8c64
to
d6b9a33
Compare
Signed-off-by: Laurent Garnier <[email protected]>
b28995b
to
9f78dad
Compare
Signed-off-by: Laurent Garnier <[email protected]>
Signed-off-by: Laurent Garnier <[email protected]>
2718ae4
to
653f0cc
Compare
This is a really awesome feature and I'm really looking forward to it. However the longer I think about the more I think it would be better if the conversion endpoint would not access existing things/items/etc. at all but these would have to be passed in. e.g. a single
The body would then be the normal DTOs like they are returned from the existing rest-api endpoints. @lolodomo I'd love to hear your thoughts about this. What do you think about this approach? Do you - like me - think this would be an improvement? |
I am OK with a new endpoint "/fileFormat". Tha's true that /items and /things endpoints are already largely cluttered. The other advantage I see with a new endpoint is that the current solution has to deal with inconsistency between thing UID in the URI and the one in the provided thing data (body). So in this case, your solution avoid this problem and I like that. You would like to remove the API that is for me the most important and the most magic, the one that will allow for a user just by calling one API without providing anything to create a file format for all his items or things currently existing in the registry. This is an easy way for the user to move to the current file format or a future file format. So I cannot agree with having no API based on the registries. Now I agree with you that we need an API that takes a ThingDTO and produces the file file format and another that takes a file format and produces a ThingDTO. It is what I introduced yesterday. To be precise, the second is currently producing a EnrichedThingDTO and I have to clarify if we should use ThingDTO or EnrichedThingDTO in both APIs. Edit: thingsDTO is appropriate. I prefer as it is now a query parameter to select the file format, rather than creating multiple sub-endpoints, one per format. It makes the API already OK for the future even if a new formats are introduced in the future. So the compromise would be:
Same principle for items. Is it a reasonable compromise ? |
Finally I think a separate API for things from registry remains better, keeping the POST APIs only for ThingDTO <-> syntax as you suggested. GET /file-format/things-from-registry |
Fair point - things should be easy for the user.
What is your idea about resolving ambiguities?
That would also work. If the body is empty ( I'm still not a hundred percent sold on that because I think making requests and posting data between endpoints is cheap and it's something that's not done very often. The benefit is that there is a clear flow of data and an arbitrary input always produces the same output. But if it's clear from the use of the endpoint that the item registry is getting accessed or that it's not getting accessed than I have no objections.
This was my idea at first, too. However I then thought about how things will documented e.g. in api explorer and as far as I know there is currently no way to document the input/output data structure based on the query parameter. (You just wrote your response while I was typing the second paragraph out so I just left everything in) Just a small suggestion: How about a sub-endpoint which returns the registry?
|
Would this allow us to get rid of the Combined with #4585, this would also open up an easier route to defining sitemaps in yaml, and switch between UI configuration, DSL and yaml in the UI. One challenge I potentially see is showing errors in the UI. For sitemaps, as the parsing is done in the UI code, it will show (to a degree, and not always perfect) where the error in the sitemap DSL is. If the whole translation is done in core, the API response should be able to indicate where the error is as well, to be able visualize it in the UI. It shouldn't just be a log entry. |
Too much complicated to achieve by a lambda user.
If the POST APIs are fully independent from our server registries, we have no conflicts to resolve. It is the role of the client to finally not request the creation of a new item/thing in the registry that already exists. I believe there is nothing new there.
That would be fine for me, Looks better than my proposal: |
Yes, that is what we are discussing in a RFC I recently opened and that I finally started to embed in that PR.
Yes, that was my intention to come to you in a second step to also consider sitemap. But I will do that in a next step and for that I would need first some explanations from you how it works today, what are the used APIs and how is triggered the code in
Yes, that is exactly my current intention started with items and things but that I can continue later with sitemaps.
That is an interesting point, in my current (basic) implementation, the API that parses the DSL syntax will return a 400 status code and the errors are logged. One thing I could do easily is to provide what we currently log as output of the API in case of invalid syntax, so that it can be displayed in UI. But I will not be able to be more clever and more precise than what currently provides the core framework in logs when the DSL file is parsed. |
I think the rest api users are only developers and even for large installations (>1k items) it's almost instant.
I fully agree 👍
That would be great. Would it then need to be logged at all? |
I think that’s a good start. We may then have to look at the errors generated by core at some point to refine the messages (e.g. the position of the error would always e useful if it can be found), but that’s a next step. Also the logs would benefit from improvements there. |
Related to #4509
Added REST APIs:
All these APIs have a parameter named "format" to request a particular output format. Of course, a syntax generator should be available for this format. Only "DSL" format is provided by this PR as this is currently our unique supported format for items and things in config files. So this parameter is set to "DSL" by default. In the future, new formats could be added and they will be automatically supported by these APIs.
All the APIs to generate syntax have a boolean parameter named "hideDefaultParameters" to hide or show the configuration parameters having the default value. They are hidden by default.