coasys / ad4m-core-deprecated

The Agent-Centric Distributed Application Meta-ontology or just: Agent-Centric DApp Meta-ontology
31 stars 4 forks source link

Subscription refactor #45

Closed kaichaosun closed 2 years ago

kaichaosun commented 2 years ago

Changes,

Fix https://github.com/perspect3vism/ad4m/issues/43

kaichaosun commented 2 years ago

Small question and also agree with Fayeed that its better if not all the callbacks are hit on a perspective mutation but only the subscriptions for that perspective. I think otherwise we may have to do some complex / filtering thing on the front end to deal with this.

I was thinking that there may be requirements for ad4m-client, to both provide same callbacks for different perspectives, and different callbacks for different perspectives, so the better solution for these requirements is let each callback decide how to behave on different perspectives based on some meta info, tags, signature or links of the perspective.

It would be good if you guys can provide more context on how this function is used in Flux. @fayeed @jdeepee

Edited, After reading more code, same callbacks for different perspectives can be optimized later when performance issue raised.

kaichaosun commented 2 years ago

Finish mapping from perspective id to linkAdded/linkRemoved callbacks. also clear the test.

kaichaosun commented 2 years ago

@lucksus I'm not concerned with broken changes actually,

I actually reading your comments on previous PR with subscription in PerspectiveProxy and decide it probably better to make all the subscription request all in client class.

lucksus commented 2 years ago

I agree with the notion of doing changes that improve the interface earlier, rather than later. But I don't regard these changes as improvements.

The discussion in the PR you linked is about how subscriptions are implemented, which can and so far is hidden from the interface Ad4mClient (and PerspectiveProxy) provides.

I think PerspectiveProxy should be the go-to interface for dealing with perspectives from the UI. In every context where we only need access to one perspective we should try to only pass down a PerspectiveProxy instead of the full Ad4mClient. Like for instance done in the PerspectiveView spec: https://github.com/perspect3vism/PIPs/blob/main/pip-0002.md This PR would already break that spec since PerspectiveViews (and several views in Perspect3ve - don't know about Flux) only have a PerspectiveProxy AND need to register callbacks. I don't see a good reason for changing that.

Also, I don't think Ad4mClient users should be forced to understand (i.e. spent more time reading docs or code) how to activate callbacks. Default should be to have them turned on. I'm actually very fine with the state of this right now. I understand the whole reason for this effort is that you try to get rid of warnings in the console that show up when a user (=UI) doesn't have the cap tokens for some perspectives. I actually think the current stat is fine already, and these warnings are not a problem but providing good feedback for anybody who tries to understand more.

If you care about adding a feature where somebody who knows about capabilities and why that warning is there and doesn't want to be bothered, can then turn it of, please go ahead but don't break things :)