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

Added Request & Response to API Spec #177

Merged
merged 2 commits into from
Feb 23, 2023
Merged

Added Request & Response to API Spec #177

merged 2 commits into from
Feb 23, 2023

Conversation

amorton
Copy link
Contributor

@amorton amorton commented Feb 23, 2023

Moved from the HTTP network spec so we can reference in the API spec, needed to lock down the responses

resolves #175

What this PR does:

Moved from the HTTP network spec so we can reference in the API spec, needed to lock down the responses

Which issue(s) this PR fixes:
Fixes #175

Checklist

  • [NA] Changes manually tested
  • [ NA] Automated Tests added/updated
  • Documentation added/updated
  • CLA Signed: DataStax CLA

Moved from the HTTP network spec so we can reference in
the API spec, needed to lock down the responses

resolves #175
@amorton amorton requested a review from a team as a code owner February 23, 2023 07:04
Copy link
Contributor

@ivansenic ivansenic left a comment

Choose a reason for hiding this comment

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

Looks good.. I am missing a option that defines if updates/writes in many commands are executed sequentially or not (ordered option or something like this).

docs/jsonapi-spec.textile Show resolved Hide resolved
h4(#commandFindResponse). find Command Response

|_. Response Element |_. Description |
| @data@ | Present with fields : @data@, @count@ and @nextPageState@ which will be @null@ if no further data is available. |
Copy link
Contributor

@ivansenic ivansenic Feb 23, 2023

Choose a reason for hiding this comment

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

  • first should second @data@ be @docs@ here?
  • second, nextPageState will be null if there are no more documents, but count and docs should never be null.. These should be empty array and zero in case nothing is found?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch on the data, and bad writing from my. Intention was only to say @nextPageState@ is null . Fixing.

h4(#commandFindOneAndUpdateResponse). findOneAndUpdate Command Response

|_. Response Element |_. Description |
| @data@ | Present with fields : @data@ only |
Copy link
Contributor

@ivansenic ivansenic Feb 23, 2023

Choose a reason for hiding this comment

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

again @docs@? what doc is returned before modification or after, unclear? Can this be controlled with an option?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed docs, the projection is controlled by the @returnDocument@ in the options, will mention that

h4(#commandFindOneResponse). findOne Command Response

|_. Response Element |_. Description |
| @data@ | Present with fields : @docs@ only |
Copy link
Contributor

Choose a reason for hiding this comment

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

so find one has no count? Is it ok that find commands have different responses?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We decided to only include fields when there is useful information, so dropped count. Honestly not sure we need count anywhere, it is only the size of the docs array.


|_. Response Element |_. Description |
| @data@ | Not present. |
| @status@ |Present with fields: @matchedCount@ count of documents that matched the @filter@, @modifiedCount@ count of document updated (may be less than match count), @upsertedId@ if a document was upserted (when @matchCount@ is zero) not present otherwise.|
Copy link
Contributor

Choose a reason for hiding this comment

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

As I understood idea is to insert a single doc with upsert in case match count is zero? Seems fine 👍

docs/jsonapi-spec.textile Show resolved Hide resolved
@amorton amorton merged commit 3541cbd into main Feb 23, 2023
@amorton amorton deleted the ajm/spec-changes branch February 23, 2023 21:36
@amorton
Copy link
Contributor Author

amorton commented Feb 26, 2023

Looks good.. I am missing a option that defines if updates/writes in many commands are executed sequentially or not (ordered option or something like this).

should be done today (monday my time)

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.

SPEC - confirm the general response format and what each command returns
3 participants