Add experimental link subscriptions - #3886
Conversation
|
|
||
| async #prepare(): Promise<void> { | ||
| try { | ||
| const [firstObject] = this.#args.objects; |
There was a problem hiding this comment.
I think ts should enforce that all objects are the same type so this is safe
| | QueryDefinition<any> | ||
| | Experiment<"2.0.8"> | ||
| | Experiment<"2.1.0"> | ||
| | Experiment<"2.17.0"> |
There was a problem hiding this comment.
This probably needs to be 2.59 and updated to the version at which we ship osdk/client. @ssanjay1 we def need to review this paradigm since we're shipping minor versions rapidly now
| "@osdk/foundry.admin": 2.70.0 | ||
| "@osdk/foundry.mediasets": 2.70.0 | ||
| "@osdk/foundry.ontologies": 2.70.0 | ||
| "@osdk/foundry.ontologies": 2.73.0 |
There was a problem hiding this comment.
Lets go ahead and bump the rest to keep in sync
| const EXPONENTIAL_BACKOFF_JITTER_FACTOR = 0.3; | ||
| const WEBSOCKET_IDLE_DISCONNECT_DELAY_MS = 15000; | ||
| const WEBSOCKET_HEARTBEAT_INTERVAL_MS = 45 * 1000; | ||
| export const EXPONENTIAL_BACKOFF_INITIAL_DELAY_MS: number = 1000; |
There was a problem hiding this comment.
Probably should move these to a shared util file instead of exporting from here
| } | ||
|
|
||
| /** @internal */ | ||
| export class LinkSubscriptionWebsocket { |
There was a problem hiding this comment.
Can we create a super class that both of these websockets now use for websocket connection/retry/heartbeat logic? I'd imagine that we can basically reuse onMessage, onOpen, initiateSubscribe, all the way upto the specific message ser/de and request payloads?
| @@ -810,9 +810,39 @@ export class ObjectSetListenerWebsocket { | |||
|
|
|||
| /** @internal */ | |||
| export function constructWebsocketUrl(baseUrl: string, ontologyRid: string) { | |||
There was a problem hiding this comment.
Yeah I think keeping these in the same utils file as above will be helpful
| ]; | ||
| } | ||
|
|
||
| export interface ChangeEvent< |
There was a problem hiding this comment.
Maybe LinkUpdate to match ObjectSetUpdate?
There was a problem hiding this comment.
Can we flatten this out too? We did the same for object set updates where youc an get multiple object updates back on one message, but the handler should be triggered once per specific update/object
| Q extends ObjectTypeDefinition, | ||
| L extends LinkTypeApiNamesFor<Q>, | ||
| > { | ||
| readonly links: readonly [L, ...ReadonlyArray<NoInfer<L>>]; |
There was a problem hiding this comment.
Can we leave this as a regular array? (NoInfer is fine). We generally try to avoid non-empty array type in this codebase
| : never; | ||
| } | ||
|
|
||
| type SubscribeToLinks = < |
There was a problem hiding this comment.
Can we add javadoc for this method. Also, will this work for updates to one to many link types backed by a FK? If not, we should document that here
| unsubscribe: () => {}, | ||
| }); | ||
|
|
||
| it("infers links and updates from the first object", () => { |
There was a problem hiding this comment.
Thank you for the type test
| vi.useRealTimers(); | ||
| }); | ||
|
|
||
| it("multiplexes subscriptions and routes update batches", async () => { |
There was a problem hiding this comment.
Could you make this description a bit more clear, like "uses one WebSocket for multiple subscriptions and sends updates to the correct listener", I was a bit confused by the wording
| L extends LinkTypeApiNamesFor<Q>, | ||
| > { | ||
| readonly updates: ReadonlyArray<Update<Q, L>>; | ||
| readonly objects: ReadonlyArray<Osdk.Instance<Q>>; |
There was a problem hiding this comment.
We should probably have this mimic the queries and actions API that take objects like here
, which probably means just using that same union| if (isDone(subscription)) return; | ||
| if (isSubscriptionDone(subscription)) return; | ||
| try { | ||
| subscription.listener.onError?.({ error, subscriptionClosed: true }); |
There was a problem hiding this comment.
Should we try catch this error call too?
There was a problem hiding this comment.
Nitty but I think the API should be (Employee, {listener, objects,links})
Invalidated by push of 5d34c94
5d34c94 to
1e71fd5
Compare
Invalidated by push of 1e71fd5
Invalidated by push of bd55880
Invalidated by push of 1311a36
Summary
subscribeToLinksAPI for subscribing to directed link changes from selected OSDK objectsUsage
This example watches two managers for changes to their direct reports.
fetchOnereturns realOsdk.Instance<Employee>values, andpeepsis the generated directed link API name from a manager to their reports.