Skip to content

[ASAP] CA-392453: Misc fixes to Java SDK #5610

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

Merged
merged 1 commit into from
May 9, 2024

Conversation

danilo-delbusso
Copy link
Member

@danilo-delbusso danilo-delbusso commented May 2, 2024

After the latest changes of the SDK, some of the API isn't matching ⬇️

  • Use Types.checkError instead of throwing a generic XenAPIException. This ensures the Types.XYZ family of exceptions is being used
  • Use @JsonValue to ensure base class objects are deserialised as a simple opaque_ref string, as opposed to a mapping of each field. This ensures the API's behaviour is unchanged.
  • Parse the results of task.getResult calls. The jsonrpc method returns value payloads of the form "value" : "<value>OpaqueRef:XYZ</value>" so we need to strip the surrounding XML

- Use `Types.checkError` instead of throwing a generic `XenAPIException`. This ensures the `Types.XYZ` family of exceptions are being used
- Use `@JsonValue` to ensure base class objects are deserialised as a simple opaque_ref string, as opposed to a mapping of each field. This ensures the API's behaviour is unchanged.
- Parse the results of `task.getResult` calls. The jsonrpc method returns value payloads of the form `"value" : "<value>OpaqueRef:XYZ</value>"` so we need to strip the surrounding XML

Signed-off-by: Danilo Del Busso <danilo.delbusso@cloud.com>
Copy link
Contributor

@kc284 kc284 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm approving the PR, you can choose whether to bother with the trivial comments (you could address them within #5595 as I expect it will need rebasing).

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

Successfully merging this pull request may close these issues.

3 participants