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

Json api add #24

Merged
merged 8 commits into from Jun 13, 2019
Merged

Json api add #24

merged 8 commits into from Jun 13, 2019

Conversation

tpurcell90
Copy link
Contributor

@tpurcell90 tpurcell90 commented Jun 13, 2019

fix #12

Adds the JSON API to solve #12.

Because of issues with pydantic #22 and #23 persist. If pydantic is updated to support these then the jsonapi file can be updated

Adding the JSON API to the spec generator, need to figure out how to add patternProperties
Removed a set function since I can't find how to add patternProperties, and some are recasting
of other types
Relationships are now added to the jsonapi file, but does not work in our tests.
If you print the pydantic schema it works as I would expect, but it is not working in
openapi.json
JSON API should now agree, outside of the bug where print CLASS.schema() and openapi.json
disagree
@tpurcell90 tpurcell90 self-assigned this Jun 13, 2019
@ltalirz
Copy link
Member

ltalirz commented Jun 13, 2019

@tpurcell90 The only check that fails is the one checking that the schema generated by the server equals the one committed in openapi.json.
I'll see whether we can merge something that Snehal is working on to see whether this "diff" can be made more simple - in the meanwhile you can probably just regenerate it.

Adding the openapi.json onto the remote branch. Before merge I'll compare with the one on master

class Link(BaseModel):
href: UrlStr
meta: Optional[dict]


class Links(BaseModel):
next: Optional[Union[UrlStr,Link]]
base_url: Optional[Union[UrlStr,Link]]
Copy link
Member

Choose a reason for hiding this comment

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

would you agree that this should be moved into an "optimade" links class?
I guess base_url is not part of the jsonapi links spec?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I'd move that into an "optimade". The json representation of Links is more general

"links": {
      "type": "object",
      "additionalProperties": {
        "$ref": "#/definitions/link"
      }
    },

Copy link
Member

@ltalirz ltalirz left a comment

Choose a reason for hiding this comment

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

thanks a lot!
just one question

Copy link
Member

@ml-evs ml-evs left a comment

Choose a reason for hiding this comment

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

lgtm

@ltalirz ltalirz self-requested a review June 13, 2019 14:42
@ltalirz ltalirz merged commit a83e420 into master Jun 13, 2019
@CasperWA CasperWA deleted the json_API_add branch November 8, 2019 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add JSON schema API
3 participants