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

result field could be a JSON schema instead of custom contentDescriptorObject #231

Closed
rmedaer opened this issue Sep 23, 2019 · 7 comments
Closed
Labels

Comments

@rmedaer
Copy link
Member

rmedaer commented Sep 23, 2019

As far as I read JSON RPC 2.0, 5. Response Object there is no rule or restriction for the result object. Indeed we can either put an integer, an object or any valid JSON (including null).

See in examples:

<-- {"jsonrpc": "2.0", "result": 19, "id": 1}

Currently in OpenRPC spec, we are waiting for a Content Descriptor Object. It doesn't define which kind of content it returns as Response result.

My proposal is quiet the same then issue #226: use a JSON Schema instead of Content Descriptor Object. Despite params field which only accept an object, an array or null, we can here put any valid JSON Schema.

Example:

{
  (...)
  "methods": [
    {
      "name": "addition",
      (...)
      "result": {
        "type": "integer"
      }
    }
  ]
}
@stale
Copy link

stale bot commented Nov 22, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 22, 2019
@stale stale bot closed this as completed Nov 29, 2019
@BelfordZ BelfordZ reopened this Nov 29, 2019
@stale stale bot removed the stale label Nov 29, 2019
@stale
Copy link

stale bot commented Jan 29, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 29, 2020
@stale stale bot closed this as completed Feb 5, 2020
@BelfordZ
Copy link
Member

@rmedaer Haven't forgot about this. Been thinking about ditching content descriptor for just plain json schema for both param and result.

Scope of change would be quite large, but the more I think about it the more it makes sense.

@BelfordZ BelfordZ reopened this Feb 12, 2020
@stale stale bot removed the stale label Feb 12, 2020
@stale
Copy link

stale bot commented Apr 12, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Apr 12, 2020
@stale stale bot closed this as completed Apr 19, 2020
@shanejonas
Copy link
Member

the separation we have with ContentDescriptors is better for code generation, and lines up with OpenAPI that we forked, having name, summary, description, schema, deprecated.

if we had just JSON Schemas for params, we'd have to have separate specs for by-name vs by-position for each parameter type with a oneOf.

params cant just be forced as only objects: type: "object", properties: { bar: { type: "string" }. As an array param the connonical json schema would be type: "array", items, and the ordering of JSON keys cant be preserved with JSON.parse.

@shanejonas shanejonas reopened this Jul 6, 2021
@stale stale bot removed the stale label Jul 6, 2021
@zcstarr
Copy link
Member

zcstarr commented Jul 6, 2021

Since this was mentioned, I'll talk about this in the context of result. With result it's much simpler, but I think there's still a lot of value add in having an explicit usage of a content descriptor ( name decoupled from type and no guesswork there). It will help with downstream consumers of the spec ( annotations code generation) , as well as make it easier for lib builders to handle one specific case contentDescriptors imho.

@stale
Copy link

stale bot commented Oct 2, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Oct 2, 2021
@stale stale bot closed this as completed Apr 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants