Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can use Trivy to scan for security misconfigurations and secrets in Infrastructure as Code files.Add a .trivyignore file to your project to customize which findings Trivy reports. |
f08cbfb to
2ac8d6a
Compare
2ac8d6a to
72f0c85
Compare
72f0c85 to
f53ddb4
Compare
f53ddb4 to
a010f31
Compare
a010f31 to
4ed0150
Compare
|
Some notes:
Otherwise looks great, thanks |
|
|
||
| To use LiveObjects, an API key must have at least the `object-subscribe` capability. With only this capability, clients will have read-only access, preventing them from publishing operations. | ||
|
|
||
| In order to create or update objects, make sure your API key includes both `object-subscribe` and `object-publish` [capabilities](/docs/auth/capabilities) to allow full read and write access. |
There was a problem hiding this comment.
This reads like you need object-subscribe to publish messages. Perhaps it should be something like
In order to create or update objects, make sure your API key includes the `object-publish` [capability](/docs/auth/capabilities). For full read and write access include both.
Or
To allow full read and write access include both the `object-subscribe` and `object-publish` [capabilities](/docs/auth/capabilities).
There was a problem hiding this comment.
Was copied from the REST API usage docs, updated both to be clear about the expected capability string, see 6fda2c3
|
|
||
| `get(RestObjectGetParams params?): Promise<Object>` | ||
|
|
||
| Reads object data from the channel. Uses the channel's root object as the entrypoint when no `objectId` is provided. Makes a request to the [`GET /channels/{channelId}/object`](/docs/liveobjects/rest-api-usage#fetch-channel-object) REST API endpoint. The return type depends on the `compact` parameter: when `compact` is `true` (default), returns a [`RestObjectGetCompactResult`](#rest-object-get-compact-result); when `compact` is `false`, returns a [`RestObjectGetFullResult`](#rest-object-get-full-result). |
There was a problem hiding this comment.
I think Uses the channel's root object should be Uses the channel object
Maybe this sentence should:
Reads object data from the channel. If no `objectId` is provided then the entire channel object is retruned.
There was a problem hiding this comment.
Same as Mike's comment about not using "root". Fixed in 6fda2c3
kaschula
left a comment
There was a problem hiding this comment.
A few comments, nothing major
| | **Compact** (default) | `compact: true` | Values-only representation without metadata. Ideal for reading data values. | | ||
| | **Non-compact** | `compact: false` | Full structure including object IDs and type metadata. Useful for debugging. | | ||
|
|
||
| **Compact format** returns the logical structure of your data as a JSON-like value. [LiveMap](/docs/liveobjects/map) instances appear as JSON objects with their entries, and [LiveCounter](/docs/liveobjects/counter) instances appear as numbers. `bytes`-typed values are returned as <If lang="javascript">`ArrayBuffer`</If><If lang="nodejs">`Buffer`</If> when using the binary protocol, or as base64-encoded strings when using the JSON protocol. `json`-typed values remain as their JSON-encoded string representation, as the client SDK cannot distinguish between a regular string and a JSON-encoded string in the compact view. |
There was a problem hiding this comment.
json-typed values remain as their JSON-encoded string representation, as the client SDK cannot distinguish between a regular string and a JSON-encoded string in the compact view
I'm not sure what this sentence is trying to express — is it "this is a restriction imposed on the SDK by the design of the REST API"? Because the REST API could conceivably return json values as inline JSON values, but chooses not to (for reasons I don't understand but which have presumably been discussed).
There was a problem hiding this comment.
I'm not sure what this sentence is trying to express — is it "this is a restriction imposed on the SDK by the design of the REST API"?
Yes, would you like it to be removed and just be "json-typed values remain as their JSON-encoded string representation"?
There was a problem hiding this comment.
Decided to remove "as the client SDK cannot distinguish between a regular string and a JSON-encoded string in the compact view" and similar line from "Data values reference" section in 6fda2c3
| </Aside> | ||
|
|
||
| <Aside data-type='further-reading'> | ||
| See [PathObject](/docs/liveobjects/concepts/path-object) for details on path-based access in client SDKs. |
There was a problem hiding this comment.
The current page arguably already describes "path-based access in client SDKs" (i.e. the REST interface of the SDK); should it say in "realtime client SDKs"?
There was a problem hiding this comment.
Updated both REST API and new REST SDK docs in 6fda2c3 to say "realtime client SDKs"
|
|
||
| ## Related types <a id="related-types"/> | ||
|
|
||
| ### RestObjectGetParams <a id="rest-object-get-params"/> |
There was a problem hiding this comment.
I was struggling to understand the heading hierarchy here — here's what Claude thinks:
- ### RestObjectGetCompactResult (h3)
- ### RestObjectGetFullResult (h3)
- ### RestLiveObject (h3)
- #### RestLiveMap (h4) ✓
- #### RestLiveMapValue (h4) — should be h5? It's a child of RestLiveMap
- ### RestObjectDataMapEntry (h3) — should be h4 or h5, it's nested under
RestLiveMapValue
- #### RestLiveObjectMapEntry (h4) — peer of RestObjectDataMapEntry but at
a different heading level
- #### RestLiveCounter (h4) — peer of RestLiveMap, this is correct
- #### RestLiveCounterValue (h4) — child of RestLiveCounter, should be h5
- ### RestObjectData (h3)
- ### PublishObjectData (h3)
- ### RestObjectPublishResult (h3)
There was a problem hiding this comment.
also the sidebar doesn't reflect the hierarchy — does it stop indenting further after a certain heading level?
There was a problem hiding this comment.
also the sidebar doesn't reflect the hierarchy — does it stop indenting further after a certain heading level?
It seems to be intended behavior of the docs website (I assume to avoid overloading the nav). It only renders two levels: ** and ***. Any other headings are only on the page itself.
I was struggling to understand the heading hierarchy here — here's what Claude thinks:
I think I added those additional headings for some types by mistake. They should all sit at the same ### level, as we're just listing all related types used by the REST SDK methods. I've changed all types to use the same ### heading in 6fda2c3 - this is consistent with other REST SDK docs, like at https://ably.com/docs/api/rest-sdk/types#rest-data-types
|
I dont think this comment has been addressed:
|
Applies suggestions from REST SDK usage docs PR [1] [1] ably/docs#3258 (comment)
As suggested in REST SDK usage docs PR [1], provide a helper for generating object IDs needed for *CreateWithObjectId operations. Constructing IDs manually is error-prone, and the SDK already has the internal machinery for this (used by the realtime client), so exposing it as a public method is straightforward. [1] ably/docs#3258 (comment)
As suggested in REST SDK usage docs PR [1], provide a helper for generating object IDs needed for *CreateWithObjectId operations. Constructing IDs manually is error-prone, and the SDK already has the internal machinery for this (used by the realtime client), so exposing it as a public method is straightforward. [1] ably/docs#3258 (comment)
… IDs As suggested in [1] the REST SDK now exposes a generateObjectId() method that handles the ID generation internally, removing the need for developers to manually hash initial values and nonces. Update the client-generated IDs section to use this helper and add the related types. See ably-js implementation in [2] for reference. [1] #3258 (comment) [2] ably/ably-js@2d973c6
Largely based on the existing usage doc for the REST API [1]. Added REST SDK docs as a separate page for two reasons: 1. There is no language selector for the shell/curl/HTTP "language", nor do I think it would look or feel nice UX-wise. It would only exist for this single page and may not be obvious to readers that they can switch it at the top to see SDK/API docs. 2. The SDK docs differ in many minor details throughout, and it would be a nightmare to maintain different versions on the same page using if-lang blocks. Where large sections are unchanged (like the procedure to generate the client-generated object ID), the doc references the corresponding REST API section instead of duplicating it. Resolves AIT-319 [1] https://ably.com/docs/liveobjects/rest-api-usage
… IDs As suggested in [1] the REST SDK now exposes a generateObjectId() method that handles the ID generation internally, removing the need for developers to manually hash initial values and nonces. Update the client-generated IDs section to use this helper and add the related types. See ably-js implementation in [2] for reference. [1] #3258 (comment) [2] ably/ably-js@2d973c6
a7b931b to
65f2a71
Compare
REST SDK docs were based on the existing "REST API usage", which still used the term "root" in some places. I've fixed both (REST SDK and REST API docs) to now only use "channel object" in 6fda2c3.
Changed in 6fda2c3. Also updated REST API usage docs to use "Full" to keep it consistent between the two.
I've removed that section from the REST SDK docs and made it clear that in the "client-generated ids" flow users should assign in the same batch. See 6fda2c3. Question: REST API docs have the same section for standalone objects. Would you like it to be removed for REST API docs too?
I've added a new |
Docs for ably/ably-js#2109.
Largely based on the existing usage doc for the REST API [1].
Added REST SDK docs as a separate page for two reasons:
do I think it would look or feel nice UX-wise. It would only exist
for this single page and may not be obvious to readers that they can
switch it at the top to see SDK/API docs.
a nightmare to maintain different versions on the same page using
if-lang blocks. Where large sections are unchanged (like the
procedure to generate the client-generated object ID), the doc
references the corresponding REST API section instead of duplicating
it.
[1] https://ably.com/docs/liveobjects/rest-api-usage
Resolves AIT-319
Description
Checklist