-
Notifications
You must be signed in to change notification settings - Fork 67
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
Updating DataStore API definition to consider ResourcesAPI structure #2368
Updating DataStore API definition to consider ResourcesAPI structure #2368
Conversation
2fe2c15
to
23d6d0c
Compare
api/openapi.yaml
Outdated
@@ -1429,10 +1429,10 @@ paths: | |||
description: "problem deleting a demo" | |||
|
|||
# Data Stores | |||
/datastores: | |||
/datastore: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Today on the Resources Manager engine, we only deal with resources in the singular form on the endpoints.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so if we add a plural form, it would leave the api almost unchanged, right? it's not hard to add that to the resource manager, do you think that would work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes sense. I'm finishing the first draft of the DataStore, and I'll add this functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issue created: #2387
@@ -1449,17 +1449,12 @@ paths: | |||
schema: | |||
type: integer | |||
default: 0 | |||
- in: query |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now, we are not explicitly putting a query
parameter on our endpoints because they are injected as it is on the queries, making our endpoints unsafe. Later we need to improve this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO we should put it here and fix the implementation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. We can open a PR to solve that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issue created: #2388
78a3589
to
9997297
Compare
Is there anything to 'show' out of this via a Loom video? |
9f7ac8a
to
62adde3
Compare
@kdhamric it is possible! I've updated the PR description with a Loom video. |
Fixing sorting fields on OpenAPI Updating OpenAPI to consider DataStore endpoint managing only one record Add endpoint in plural
62adde3
to
07e8bc5
Compare
…2368) Fixing sorting fields on OpenAPI Updating OpenAPI to consider DataStore endpoint managing only one record Add endpoint in plural
…2368) Fixing sorting fields on OpenAPI Updating OpenAPI to consider DataStore endpoint managing only one record Add endpoint in plural
…2368) Fixing sorting fields on OpenAPI Updating OpenAPI to consider DataStore endpoint managing only one record Add endpoint in plural
…2368) Fixing sorting fields on OpenAPI Updating OpenAPI to consider DataStore endpoint managing only one record Add endpoint in plural
* Updating DataStore API definition to consider ResourcesAPI structure (#2368) Fixing sorting fields on OpenAPI Updating OpenAPI to consider DataStore endpoint managing only one record Add endpoint in plural * Adding datastoreresource package * Update internal references to use datastoreresource package * Isolate database resources on each test * Adding behavior for no datastore scenario * Fixing rebase problems * Fixing another rebase problem * Updating DataStore API definition to consider ResourcesAPI structure (#2368) Fixing sorting fields on OpenAPI Updating OpenAPI to consider DataStore endpoint managing only one record Add endpoint in plural * Fixing merge * Adding documentation for delete operation * Improving cleanup code * Updating datastore API * Adding CLI generated code * wip * wip * Fixing CLI * deprecated old commands * Adding wrongly deleted file * simplifying the resource actions creation steps --------- Co-authored-by: Oscar Reyes <oscar-rreyes1@hotmail.com>
… CLI commands (#2414) * Updating DataStore API definition to consider ResourcesAPI structure (#2368) Fixing sorting fields on OpenAPI Updating OpenAPI to consider DataStore endpoint managing only one record Add endpoint in plural * Add Data store as resource on Backend (#2385) * feat(frontend): add data store as resource (#2423) * Add data store as resource on CLI (#2422) * Updating DataStore API definition to consider ResourcesAPI structure (#2368) Fixing sorting fields on OpenAPI Updating OpenAPI to consider DataStore endpoint managing only one record Add endpoint in plural * Adding datastoreresource package * Update internal references to use datastoreresource package * Isolate database resources on each test * Adding behavior for no datastore scenario * Fixing rebase problems * Fixing another rebase problem * Updating DataStore API definition to consider ResourcesAPI structure (#2368) Fixing sorting fields on OpenAPI Updating OpenAPI to consider DataStore endpoint managing only one record Add endpoint in plural * Fixing merge * Adding documentation for delete operation * Improving cleanup code * Updating datastore API * Adding CLI generated code * wip * wip * Fixing CLI * deprecated old commands * Adding wrongly deleted file * simplifying the resource actions creation steps --------- Co-authored-by: Oscar Reyes <oscar-rreyes1@hotmail.com> * fix ingester dsRepo * chore(docs): update data store docs and examples (#2429) * removing test deletion * fixing provisioning file * fixing provisioning file --------- Co-authored-by: Jorge Padilla <jorge.esteban.padilla@gmail.com> Co-authored-by: Oscar Reyes <oscar-rreyes1@hotmail.com>
… CLI commands (#2414) * Updating DataStore API definition to consider ResourcesAPI structure (#2368) Fixing sorting fields on OpenAPI Updating OpenAPI to consider DataStore endpoint managing only one record Add endpoint in plural * Add Data store as resource on Backend (#2385) * feat(frontend): add data store as resource (#2423) * Add data store as resource on CLI (#2422) * Updating DataStore API definition to consider ResourcesAPI structure (#2368) Fixing sorting fields on OpenAPI Updating OpenAPI to consider DataStore endpoint managing only one record Add endpoint in plural * Adding datastoreresource package * Update internal references to use datastoreresource package * Isolate database resources on each test * Adding behavior for no datastore scenario * Fixing rebase problems * Fixing another rebase problem * Updating DataStore API definition to consider ResourcesAPI structure (#2368) Fixing sorting fields on OpenAPI Updating OpenAPI to consider DataStore endpoint managing only one record Add endpoint in plural * Fixing merge * Adding documentation for delete operation * Improving cleanup code * Updating datastore API * Adding CLI generated code * wip * wip * Fixing CLI * deprecated old commands * Adding wrongly deleted file * simplifying the resource actions creation steps --------- Co-authored-by: Oscar Reyes <oscar-rreyes1@hotmail.com> * fix ingester dsRepo * chore(docs): update data store docs and examples (#2429) * removing test deletion * fixing provisioning file * fixing provisioning file --------- Co-authored-by: Jorge Padilla <jorge.esteban.padilla@gmail.com> Co-authored-by: Oscar Reyes <oscar-rreyes1@hotmail.com>
This PR updates the OpenAPI definition to consider the DataStore API on the resources API architecture.
Note: We are only running
make generate
to change the types on the backend PRs (#2385), just to avoid breaking things on this PR.Loom: https://www.loom.com/share/0870ffdc991b4f37b8e35faf281a6605
Changes
api/openapi.yaml
definitionsFixes
Checklist