itpp-labs / pos-addons

Odoo POS Addons
https://itpp.dev
MIT License
361 stars 582 forks source link

/pos_longpolling/update #645

Open blaggacao opened 6 years ago

blaggacao commented 6 years ago

https://github.com/it-projects-llc/pos-addons/blob/fe1e50ce94efd6412b5444a968589eca35910ac4/pos_longpolling/controllers/pos_longpolling_controller.py#L17

At the risk of overlooking something, I'm trying to get my head around this endpoint and weather it would not be feasible to achieve the same via /longpolling/send...

I'm trying to think of a satellite Bus (motivation: https://github.com/it-projects-llc/pos-addons/issues/644) to support true offline order syncing, and I was just thinking to implement /longpolling/satellite/send as a standard means to share messages across the satellite subscribers... So I was thinking of refactoring this endpoint into /longpolling/send to get things more aligned and do transparent switches between satellite bus and main server bus.

What do you guys think? @yelizariev @GabbasovDinar

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/55992787-pos_longpolling-update?utm_campaign=plugin&utm_content=tracker%2F2289114&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F2289114&utm_medium=issues&utm_source=github).
blaggacao commented 6 years ago

I've stumbled over http://blog.fanout.io/2013/03/19/pollymer-a-general-purpose-long-polling-library/ and am somewhat inclining to use it as a library. Company behind seems rocksolid and it looks rather stable (at first sight). Actually, I have difficulty to wrap my head around the Odoo bus implementation details and think this should get out of the way relying on a well tested library so that we can spend our time on the business logic and exciting new functionality and do not need to treat low level details as CORS or JSON-P handling, randomness delay, failure handling, re connection, timeouts implementations, etc...

blaggacao commented 6 years ago

I'm advancing, here I simulate a hard server shutdown (2x CTRL+C):

The very same long polling would be able to detect such disconnection, so I guess this update polling loop can be eliminated for the sake of simplifying the code conveying it's task to the logpolling/poll or longpolling/satellite/poll itself.

Please note: it is not detected as an error (error trigger) but as a finish (finish trigger) without content.

longpolling

yelizariev commented 6 years ago

built-it longpolling ignores any error from longpolling request. I believe it's not possible to catch all network errors and the only way to use periodic ping requests which we made in pos_longpolling. So, I'm not sure that we need to change something about this part.

With regard to new controllers, let's discuss in or after #644

blaggacao commented 6 years ago

WIP, but you can see how the sync icon is read and pollymer (gracefully) does 2 retries (3 tries in total) with increasing latency, first 1860ms then 2861ms after a 2xCTRL+C. As the connection effectively drops, the XMLHttpRequest will (hopefully?!?) fail on any type of network error.

This gracefulness is interesting for let's say load-balancer switching / rolling upgrade operations or similar stuff which might occur in production but should not tear down the restaurant of the customer... :smile:

screenshot from 2018-03-11 22-31-31

blaggacao commented 6 years ago

:smile: http://nimb.ws/RKnLxT

yelizariev commented 6 years ago

If it allows detecting problems more quickly, that could be good. We just need be absolutely sure, that it always catches network problems

blaggacao commented 6 years ago

It does! :) Even a graceful termination (which returns HTTP code=0) is detected as a failure...

blaggacao commented 6 years ago

I'm, by now, able to dynamically create satellite endpoints, (there can be as many as makes sense), each of which would create a independent UI widget... I'm now struggling to make multi_session speak over the this "bus infrastructure". I'm not sure whether it's better to let broadcast a message through a single channel and let the client decide based on originator id weather do discard the broadcasted message (if sent by himself) or keep the server side implementation and leave multiple channels so that every client has his own channel which makes things a little less transparent, server side.

Especially, if you want to audit what traffic has gone through a specific multi session, you need to clear the channel with LIKE "pos.multisession-sessionIF%", which cannot be easily done in odoo reporting (for grouping)..

keadanis commented 4 years ago

Hi blaggacao! How was your tests? Did you come to any conclusion on that road?