openbmc / phosphor-rest-server

REST server that transposes dbus interfaces to REST
Apache License 2.0
4 stars 11 forks source link

The streaming support for obmc-rest. #15

Closed shgoupf closed 8 years ago

shgoupf commented 8 years ago

Changes: 1) The main idea of this change is to have a streaming path as below: dbus signal -> obmc-rest capture the dbus signal -> obmc-rest notify the client of the signal receiving. 2) Replace rocket with gevent WSGI server to support multiple async accesses. 3) Use gevent queue to notify the dbus signal receiving. 4) The uri to the streaming should be in the form as below: https:////stream/


This change is Reviewable

anoo1 commented 8 years ago

hi @shgoupf , would you rebase your change and resubmit?

shgoupf commented 8 years ago

Hi @anoo1 , rebase done. Let me know if there is any problem with this change.

anoo1 commented 8 years ago

Thanks @shgoupf . @bradbishop will provide you with some feedback.

shgoupf commented 8 years ago

@anoo1 and @bradbishop , please note that the feature added by this pull request requires gevent, which is added to the yocto framework in another pull request https://github.com/openbmc/openbmc/pull/161

bradbishop commented 8 years ago

On Mon, 2016-06-06 at 17:16 -0700, shgoupf wrote:

@anoo1 and @bradbishop , please note that the feature added by this pull request requires gevent, which is added to the yocto framework in another pull request openbmc/openbmc#161

We are working on bringing in the oe layers that have all these recipes so we don't have to maintain them in the phosphor layer. So we can probably close pull #161. Or leave it open until meta-oe and meta-virtualization get added as a reminder.

I'm pretty sure when I tried this out awhile back the signal loop quit after streaming one signal message. My expectation was that signals would continue to stream until the connection was closed. Is it possible to do that? Or maybe it does work but I am remembering wrong?

thx - brad

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

shgoupf commented 8 years ago

@bradbishop The signal loop should be always running. I have tested it for my local runs. So please let me know what's going on for your test.

shgoupf commented 8 years ago

@bradbishop Sorry that after I double checked the code, I realized the signal loop would stop after received 1 dbus signal. I tried to test my fix to this problem, however since I volunteered to this task almost half a year ago, I am not able to access the test machine for now. Seems like the test machine (palmetto) is not allowed to flash, which is required to add gevent/greenlet for testing. So Brad, please provide me with a test machine to test this streaming feature. Or if you already have an environment setup for testing, please just comment out the code in line 834 (self.snooping = False) and proceed to test. This should let the signal loop running even after message received.

shgoupf commented 8 years ago

@bradbishop Hey Brad. I did setup the test env in our local lab. However, it turns out that the current solution (without websocket, only pure gevent/greenlet with Rest API) is not able to continuously let client receive message (even if we can keep server side signal loop running after catch dbus message, we cannot receive the message during runtime from client side, because the Rest client will only get response once for one request). I vaguely remember that websocket can do this by keeping the connection and receive message continuously, but we abandoned websocket based on your advice in early this year.

Technically if we do want to keep receiving signals, for current solution, every time the client received the signal, it should issue another request from client side to keep receiving.

How do you think about this?

I updated the pull request for some minor changes to align with the current code base.

williamspatrick commented 8 years ago

Repository migrated to gerrit server. Please reopen as a pending commit-set over there.