Implement SEARCH notifications for the ConfigDB #368
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
djnewbould
previously approved these changes
Nov 13, 2024
The generic Notify interface does not want to know about the ConfigDB's model object. Whether an auth interface is part of the generic interface or not I'm not sure yet; for now treat it as service-specific.
The HTTP API will accept this endpoint either with or without the trailing slash. For the notify interface I am going to be more strict and require it, as this will make relative URLs work correctly when we implement SEARCH.
This allows subscription to a whole folder of URLs, with optional filtering.
This is not complete: * There is no filter support. * There is no checking of ACLs.
Collecting up the list of initial updates on the client side is proving annoying. This will get harder when we handle permission changes and other things which cause full updates to be sent at other times. We are using a WebSocket; there is no particular limit to the length of a message. There is a small possibility that we will hit some JSON parser limit but in that case the client probably shouldn't be using SEARCH in the first place.
This method of sending updates is much easier to work with. Require a response for the parent resource in every full update.
With the current ACL system a 403 always applies to the parent and all children. We only check ACLs when we get a child update but then we need to convert that into a parent update. The spec says we must not follow a no-access update with a child update; we need to convert that child update to a full update. We also need to pretend that we've just sent a no-access update initially. Implement `withState`, which is the operator I usually want `scan` to be. This makes a distinction between the state value and the pipeline output value.
This does not attempt to be intelligent about the initial fetch; it fetches everything and filters in Rx. This could be improved but would need a way to translate arbitrary JMP filters into Postgres syntax.
I'm not sure what will happen if an app is deleted... I'm not sure that's a case worth handling, though. This means reworking search_full as a plain Promise-returning function, and adding an `asyncState` operator to accept Promises to state. I don't think trying to force these Promises into Observables would be helpful; something like RxJava's Single would be best, but with only Observable there is always the question of how to handle multiple results.
These are supposed to be URLs down from the top of the service's URL namespace; this means the initial `v1` is required. It would be better to refactor so that the REST API and the notify API were provided by the same URL definition.
amrc-benmorrow
force-pushed
the
bmz/ws-search
branch
from
d3ef2d5
to
bee0fe1
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Specify a new change-notify request, SEARCH. This allows a client to request notifications for changes to all URLs under a given path. The intended use-case is for watching
app/:app/object/to track updates to configs we are interested in.Some apps (notably the edge-sync operator) will filter out many objects. Support this in the API by allowing the client to apply a filter to the notifications. Filtered-out objects are not returned and updates to them are ignored, however the client is kept informed when objects enter or leave the set of interest.