Skip to content
This repository has been archived by the owner on Sep 12, 2023. It is now read-only.

Don't send a list of all CFDs for updating the UI #2906

Closed
da-kami opened this issue Sep 7, 2022 · 5 comments
Closed

Don't send a list of all CFDs for updating the UI #2906

da-kami opened this issue Sep 7, 2022 · 5 comments

Comments

@da-kami
Copy link
Contributor

da-kami commented Sep 7, 2022

Proposal:

We don't send data over the update feed, but only tell the UI that something changed, and the UI can then decide what data is to be loaded.
I think this will result in a more efficient user interface and experience, because the UI (i.e. the consumer) decides what is needed according to what is displayed. This allows adding e.g. pagination in a simple way.
However, it will require somewhat more code, because we will have to add endpoints to fetch data.

For offer and quote I would apply the same principle, but it is not as critical because there is no ever growing list of quotes/offers.

@klochowicz
Copy link
Contributor

if we limit to only sending open CFDs on the feed, then this becomes a non-issue with so much less code.
Closed CFDs could be requested on demand, paginated.

Complicating this interface would as a side-effect introduce the need of actually having UI tests, something that we got away without so far, and it would be very unfortunate if we had to add them now...

@bonomat
Copy link
Collaborator

bonomat commented Sep 8, 2022

Cc @holzeis : we had a similar discussion on a different ticket. Maybe you want to state your opinion here ?

@holzeis
Copy link
Contributor

holzeis commented Sep 8, 2022

Ok, I just looked over the code and yeah it looks pretty over engineered, that the initial CFDs are also published as event. (because, we always publish an initiated event when the actor starts, so that all CFDs get published - it's fabricated) What's wrong with a simple Rest API 😝 In general though I have the feeling that the CFD model is far to big. We should think about modelling events (e.g. rollover event, settlement even, etc.) - although I am generally in favor of submitting events with payload.

Regarding the discussion I had with @bonomat. I think our feed api should be more flexible on what can be subscribed to e.g. /api/feed?offers,symbol=XBTUSD.

As mentioned having a simple REST api for e.g. fetching CFDs makes a lot things easier and stupid. (pagination, versioning, debugging, API documentation, etc.). However, I think both can co-exist, we can have a REST API for fetching CFDs (getting rid of the internal initiated event) and still send updates if something changed.

The decision if we should provide a payload in the event or not is a bit more diverse. I think we should firstly look into the CFD model in general to see if that big model can't be split up in smaller ones - which take less space in the event payload.

I think a quick improvement would already be if we don't always publish all events but only those which have actually changed (if we don't do that already - haven't checked the code).

@da-kami
Copy link
Contributor Author

da-kami commented Sep 8, 2022

[snip]
I think we should firstly look into the CFD model in general to see if that big model can't be split up in smaller ones - which take less space in the event payload.

Might be relevant: #2555
This is however more a discussion about the model in general, we would still have to decide how we reflect this in the http API.

@bonomat
Copy link
Collaborator

bonomat commented Sep 12, 2023

This project is unmaintained

@bonomat bonomat closed this as not planned Won't fix, can't repro, duplicate, stale Sep 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants