CP-44477 Add Typescript bindings for XenAPI #5623
Conversation
Signed-off-by: Fei Su <fei.su@cloud.com>
|
Needs feedback from the interfaces team @kc284 @danilo-delbusso |
I agree that the lack of an intuitive API with Python approach, particularly when the user uses it within IDEs, can't auto pop-up code completion as the methods are dynamic. However, with the help of Python, many examples and code snippets are easily available online for developers to refer to, which will definitely speed up the development process. |
I can't say I have used the Python API, but - if it is as you say - then, in principle, it could be improved with forwarding stubs generated from the JSON backend. I did this for something else as of very recently, which features autocomplete: |
| headers: { "content-type": "application/json" }, | ||
| body: JSON.stringify(request), | ||
| }); | ||
| if (response.status === 200) { |
There was a problem hiding this comment.
we cannot rely on the response status in jsonrpc and xapi.
see for instance this failed task.get_result call. the call has failed, but we get a 200 status
> POST /jsonrpc HTTP/1.1
> Host: 10.71.144.44
> Content-Type: application/json
> User-Agent: insomnia/9.0.0
> Accept: */*
> Content-Length: 181
| {
| "id": 1,
| "method": "task.get_result",
| "jsonrpc": "2.0",
| "params": [
| "OpaqueRef:123",
| "OpaqueRef:123"
| ]
| }
* TLSv1.2 (IN), TLS header, Supplemental data (23):
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< content-length: 129
< connection: keep-alive
< cache-control: no-cache, no-store
< content-type: application/json
< Access-Control-Allow-Origin: *
< Access-Control-Allow-Headers: X-Requested-With
* Received 129 B chunk
* Connection #0 to host 10.71.144.44 left intact
Ideally we'd have an error type for xapi exceptions (can be a generic one), and one for HTTP errors.
There was a problem hiding this comment.
To add a bit to the above comment, we should check whether the response Json contains Error and if it does throw an exception containing the Message and Data, otherwise the user would have to do that for every call.
There was a problem hiding this comment.
since this is the first time that you're adding this library, please also add eslint so we can keep this codebase clean :)
|
|
||
| For more example usage, please find in tests folder. | ||
| ``` | ||
| $ git clone git@github.com:acefei/xen-api-ts.git |
There was a problem hiding this comment.
To make things simpler for the users, we're trying to minimise the number of places where people can find xenserver-sdk/xen-api documentation and help. You may want to consider moving your examples (importing the whole git history if you want) to a new folder typescript within this repo https://github.com/xenserver/xenserver-samples which is also linked from the official XenServer developer documentation. Let me know and I'll add you to the maintainers list.
|
Thanks all for your comments, that is really helpful, I will update this in next OIL day. |
Signed-off-by: Fei Su <fei.su@cloud.com>
acefei
force-pushed
the
private/feis/CP-44477
branch
from
a8e9c8a to
4e09cf9
Compare
|
@danilo-delbusso This is in need of a review, is that correct? |
|
If I remember well there were some of the requested changes still missing from the latest push, will need to re-check, but, yes, review is needed. |
|
Thanks! If there have been outstanding comment for so long, then I think it's better to close it. When there's an impetus to continue with the work, it can be reopened. |
In this PR, I implemented the Typescript bindings for XenAPI whose usage is almost identical to the Python XenAPI, except it's asynchronous and requires async/await.
CC @Zhengchai @qinzhang22