Skip to content
This repository has been archived by the owner on Jul 28, 2023. It is now read-only.

Introduce root endpoints' schemas #321

Merged
merged 8 commits into from
Sep 6, 2019

Conversation

yeralin
Copy link
Contributor

@yeralin yeralin commented Aug 21, 2019

Summary

Addresses #289
Adds schemas that describe request and response payloads for "root" endpoints.

Details and comments

I found couple problems while preparing this PR:

fields.Dict(keys=fields.Str(), values=fields.Float())

They discussed the implementation of this feature under marshmallow-code/marshmallow#483
Because of this limitation schemas like HubsResponseSchema and CircuitResponseSchema can only be deserialized partially.

Also because of this issue, marshmallow-jsonschema package cannot generate a proper JSON schema for nested dicts, and rendered as <not serializable>:
image

TL;DR we need to upgrade Marshmallow version to 3.0, if we want to fully deserialize schemas like HubsResponseSchema and CircuitResponseSchema

  • Deserialization of HubsResponseSchema will always fail bc @bind_schema does not allow passing many = True

The issue was raised under qiskit-terra repo: Qiskit/qiskit#3021
If we were to create a model for HubsResponseSchema using @bind_schema decorator, and then use it to deserialize hubs response payload, the deserialization will always fail with ModelValidationError. The reason is bc the payload is of the following format: [<payload>] i.e. the payload is wrapped in an array. This could be easily fixed by passing many = True while creating an instance of HubsResponseSchema(many=True), but the instance is created internally in @bind_schema decorator, and we cannot pass any arguments inside.

@yeralin yeralin self-assigned this Aug 21, 2019
@yeralin yeralin added the type: question Further information is requested label Aug 22, 2019
@diego-plan9 diego-plan9 removed the type: question Further information is requested label Sep 5, 2019
@diego-plan9
Copy link
Member

Following up on HubsResponseSchema: let's decouple the issue with that particular response in order to move forward, as at this point in time there is no need for worrying about serialization and we can handle that response via other means.

Can you make HubsResponseSchema a dummy schema (with no fields), and add a small note in the class to remind us that it needs to be treated slightly differently for the time being?

@diego-plan9 diego-plan9 merged commit 4a0c29f into Qiskit:master Sep 6, 2019
@yeralin yeralin deleted the introduce-other-schemas branch September 6, 2019 13:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants