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.

Sign up for GitHub

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

Jump to bottom

Refine GET /multihash/<multihash> response for ndjson #8

Closed
masih opened this issue Jan 26, 2023 · 0 comments · Fixed by #10
Closed

Refine GET /multihash/<multihash> response for ndjson #8

masih opened this issue Jan 26, 2023 · 0 comments · Fixed by #10
Labels
P2

Comments

@masih
Copy link
Member

masih commented Jan 26, 2023 •
edited
Loading

The response structure used by IPNI for GET /multihash/<multihash> is optimised for batch find: it includes the multihash that is being looked up as base64 encoded string, along with a nested array of results.

I believe the rationale has been to use the same response model for both batch find, and single find request. For non-streaming responses this is fine; however, for responses streamted using new-line delimited json (i.e. ndjson) it becomes suboptimal:

  • in ndjson each response needs to be a JSON object.
  • because the results for a single mulithash are nested in an array, to stream the response using the current model one has to repeat the multihash that is being looked up in every streamed response.

This is less than optimum considering the vast majority of traffic received by cid.contact is for explicit multihash lookups.

Consider spec changes for a more optimal response structure for single multihash lookup, such that:

  • the original multihash looked up via GET request is not repeated in the response or optionally included.
  • the response model is preferably made up of a series of self contained JSON objects optimised for the streaming case, which hopefully becomes the default/preferred response media type when Accept request header permits it.
@masih masih added the P2 label Jan 26, 2023
@masih masih mentioned this issue Jan 27, 2023
Merged
masih added a commit that referenced this issue Jan 30, 2023
@masih
Propose a concise ndjson response and add OpenAPI specification
9f47925 
The majority of IPNI clients use the single lookup APIs, either
`/cid/<cid>` or `/multihash/<multihash>`, instead of the batch-find API.
All three of the APIs use a uniform response schema, which includes the
queried multihash directly in it.

In a case where the request accepts streaming response, using the
existing uniform response model becomes inefficient: a response may be
flushed multiple times and as is each response would have to include the
original multihash.

The changes here propose a new trimmed-down response type when the
request accepts `application/x-ndjson`, made-up of the inner provider
record object only for the single multihash lookup APIs. This avoids the
need for including the original multihash in every streamed response.

Additionally, document the OpenAPI specification for the IPNI HTTP APIs
for better human readability.

Fixes #8
@masih masih mentioned this issue Jan 30, 2023
Merged
masih added a commit that referenced this issue Jan 30, 2023
@masih
Propose a concise ndjson response and add OpenAPI specification
bb2b9a4 
The majority of IPNI clients use the single lookup APIs, either
`/cid/<cid>` or `/multihash/<multihash>`, instead of the batch-find API.
All three of the APIs use a uniform response schema, which includes the
queried multihash directly in it.

In a case where the request accepts streaming response, using the
existing uniform response model becomes inefficient: a response may be
flushed multiple times and as is each response would have to include the
original multihash.

The changes here propose a new trimmed-down response type when the
request accepts `application/x-ndjson`, made-up of the inner provider
record object only for the single multihash lookup APIs. This avoids the
need for including the original multihash in every streamed response.

Additionally, document the OpenAPI specification for the IPNI HTTP APIs
for better human readability.

Fixes #8
masih added a commit that referenced this issue Jan 30, 2023
@masih
Propose a concise ndjson response and add OpenAPI specification
e1184fe 
The majority of IPNI clients use the single lookup APIs, either
`/cid/<cid>` or `/multihash/<multihash>`, instead of the batch-find API.
All three of the APIs use a uniform response schema, which includes the
queried multihash directly in it.

In a case where the request accepts streaming response, using the
existing uniform response model becomes inefficient: a response may be
flushed multiple times and as is each response would have to include the
original multihash.

The changes here propose a new trimmed-down response type when the
request accepts `application/x-ndjson`, made-up of the inner provider
record object only for the single multihash lookup APIs. This avoids the
need for including the original multihash in every streamed response.

Additionally, document the OpenAPI specification for the IPNI HTTP APIs
for better human readability.

Fixes #8
@masih masih closed this as completed in #10 Jan 30, 2023
masih added a commit that referenced this issue Jan 30, 2023
@masih
Propose a concise ndjson response and add OpenAPI specification
c04bdff 
The majority of IPNI clients use the single lookup APIs, either
`/cid/<cid>` or `/multihash/<multihash>`, instead of the batch-find API.
All three of the APIs use a uniform response schema, which includes the
queried multihash directly in it.

In a case where the request accepts streaming response, using the
existing uniform response model becomes inefficient: a response may be
flushed multiple times and as is each response would have to include the
original multihash.

The changes here propose a new trimmed-down response type when the
request accepts `application/x-ndjson`, made-up of the inner provider
record object only for the single multihash lookup APIs. This avoids the
need for including the original multihash in every streamed response.

Additionally, document the OpenAPI specification for the IPNI HTTP APIs
for better human readability.

Fixes #8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
P2
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Propose a concise ndjson response and add OpenAPI specification ipni/specs
1 participant
@masih