* 回复:回复:[PATCH phosphor-rest-server] The streaming support for obmc-rest. [not found] <OF66059493.03804909-ON00257F49.000380B3-1454027895800@LocalDomain> @ 2016-01-29 0:49 ` Peng Fei BG Gou [not found] ` <201601290049.u0T0nquh003020@d23av02.au.ibm.com> 1 sibling, 0 replies; 8+ messages in thread From: Peng Fei BG Gou @ 2016-01-29 0:49 UTC (permalink / raw) To: Peng Fei BG Gou; +Cc: openbmc, OpenBMC Patches, Cyril Bur [-- Attachment #1: Type: text/plain, Size: 9335 bytes --] And the white space you saw in the email thread and coding is probably caused by the mixing use of tab and white space. The original code uses tab for indent, while I prefer to use 4 white spaces for indent. I will discuss with Brad regarding the indent convention for our code. Will fix it up if we strictly need tab in our project. 在 2016年1月29日,上午8:38:15,"Peng Fei BG Gou" <shgoupf@cn.ibm.com> 写道: Python is indent based languang, so the function will fail if we have incorrect indenting. I have tested this in real bmc machine so I believe the indenting should be fine for now. Please let Brad review this change since he is familiar with Python. 在 2016年1月29日,上午8:24:58,"Cyril Bur" <cyrilbur@gmail.com> 写道: On Thu, 28 Jan 2016 02:00:37 -0600 OpenBMC Patches wrote: > From: shgoupf > Hi Peng, So I'm don't really know python all that well but I do believe this language is white space sensitive... I'll let a pythoner respond about the rest... > 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/ > --- > obmc-rest | 115 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 104 insertions(+), 11 deletions(-) > mode change 100644 => 100755 obmc-rest > > diff --git a/obmc-rest b/obmc-rest > old mode 100644 > new mode 100755 > index c6d2949..481dafa > --- a/obmc-rest > +++ b/obmc-rest > @@ -3,7 +3,9 @@ > import os > import sys > import dbus > +import gobject > import dbus.exceptions > +import dbus.mainloop.glib > import json > import logging > from xml.etree import ElementTree > @@ -14,6 +16,10 @@ from OpenBMCMapper import Mapper, PathTree, IntrospectionNodeParser, ListMatch > import spwd > import grp > import crypt > +import threading > +import gevent > +from gevent.pywsgi import WSGIServer > +from gevent.queue import Queue > > DBUS_UNKNOWN_INTERFACE = 'org.freedesktop.UnknownInterface' > DBUS_UNKNOWN_METHOD = 'org.freedesktop.DBus.Error.UnknownMethod' > @@ -59,12 +65,13 @@ def makelist(data): > > class RouteHandler(object): > _require_auth = makelist(valid_user) > - def __init__(self, app, bus, verbs, rules): > + def __init__(self, app, bus, verbs, rules, skips = []): > self.app = app > self.bus = bus > self.mapper = Mapper(bus) > self._verbs = makelist(verbs) > self._rules = rules > + self._skips = skips > > def _setup(self, **kw): > request.route_data = {} > @@ -79,7 +86,7 @@ class RouteHandler(object): > return getattr(self, 'do_' + request.method.lower())(**kw) > > def install(self): > - self.app.route(self._rules, callback = self, > + self.app.route(self._rules, callback = self, skip = self._skips, > method = ['GET', 'PUT', 'PATCH', 'POST', 'DELETE']) > > @staticmethod > @@ -108,6 +115,58 @@ class RouteHandler(object): > return None > raise > > +class SignalHandler(RouteHandler): > + verbs = ['GET'] > + rules = '/stream/' > + > + def __init__(self, app, bus): > + super(SignalHandler, self).__init__( > + app, bus, self.verbs, self.rules) > + > + def find(self, path, signal): > + busses = self.try_mapper_call(self.mapper.get_object, > + path = path) > + for items in busses.iteritems(): > + s = self.find_signal_on_bus(path, signal, *items) > + if s: > + return s > + > + abort(404, _4034_msg %('signal', 'found', signal)) > + > + def setup(self, path, signal): > + request.route_data['map'] = self.find(path, signal) > + > + def do_get(self, path, signal): > + body = Queue() > + dsignal = DbusSignal(bus, request.route_data['map'][0], > + request.route_data['map'][1], path) > + dsignal.onData(body.put) > + dsignal.onFinish(lambda: body.put(StopIteration)) > + dsignal.signalSnooping() > + return body > + > + @staticmethod > + def find_signal(signal, signals): > + if signals is None: > + return None > + > + signal = find_case_insensitive(signal, signals.keys()) > + if signal is not None: > + return signal > + > + def find_signal_on_bus(self, path, signal, bus, interfaces): > + obj = self.bus.get_object(bus, path, introspect = False) > + iface = dbus.Interface(obj, dbus.INTROSPECTABLE_IFACE) > + data = iface.Introspect() > + parser = IntrospectionNodeParser( > + ElementTree.fromstring(data), > + intf_match = ListMatch(interfaces)) > + for x,y in parser.get_interfaces().iteritems(): > + s = self.find_signal(signal, > + y.get('signal')) > + if s: > + return (x,s) > + > class DirectoryHandler(RouteHandler): > verbs = 'GET' > rules = '/' > @@ -715,7 +774,8 @@ class RestApp(Bottle): > self.install(JSONPlugin(**json_kw)) > self.install(JsonApiErrorsPlugin(**json_kw)) > self.install(AuthorizationPlugin()) > - self.install(JsonApiResponsePlugin()) > + self.json_response_plugin = JsonApiResponsePlugin() Indenting? > + self.install(self.json_response_plugin) > self.install(JsonApiRequestPlugin()) > self.install(JsonApiRequestTypePlugin()) > > @@ -726,6 +786,7 @@ class RestApp(Bottle): > > def create_handlers(self): > # create route handlers > + self.signal_handler = SignalHandler(self, self.bus) > self.session_handler = SessionHandler(self, self.bus) > self.directory_handler = DirectoryHandler(self, self.bus) > self.list_names_handler = ListNamesHandler(self, self.bus) > @@ -736,6 +797,11 @@ class RestApp(Bottle): > self.instance_handler = InstanceHandler(self, self.bus) > > def install_handlers(self): > + # Skip json response for signal handler because it requires to > + # return a gevent iterable which cannot be handled by json > + # response plugin > + self.signal_handler._skips = [self.json_response_plugin] Indenting? > + self.signal_handler.install() > self.session_handler.install() > self.directory_handler.install() > self.list_names_handler.install() > @@ -766,21 +832,48 @@ class RestApp(Bottle): > parts = filter(bool, path.split('/')) > request.environ['PATH_INFO'] = '/' + '/'.join(parts) + trailing > > +class DbusSignal(): > + def __init__(self, bus, dbus_interface, signal_name, path): > + # Register the dbus recieve handler > + bus.add_signal_receiver(self.signalReciever, > + dbus_interface = dbus_interface, > + signal_name = signal_name, > + path = path) > + > + self.snooping = True > + > + def signalReciever(self, msg): > + self.send("Recieved message: %s" % msg) > + self.snooping = False > + > + def onData(self, send): > + self.send = send > + > + def onFinish(self, f): > + self.finish = f > + > + def signalSnooping(self): > + while self.snooping: > + mainloop = gobject.MainLoop() > + gevent.sleep(1) > + gobject.timeout_add(1, mainloop.quit) > + mainloop.run() > + > + self.finish() > + > if __name__ == '__main__': > log = logging.getLogger('Rocket.Errors') > log.setLevel(logging.INFO) > log.addHandler(logging.StreamHandler(sys.stdout)) > > + dbus.mainloop.glib.DBusGMainLoop(set_as_default=True) Indenting? > bus = dbus.SystemBus() > app = RestApp(bus) > + ? > default_cert = os.path.join(sys.prefix, 'share', > os.path.basename(__file__), 'cert.pem') > > - server = Rocket(('0.0.0.0', > - 443, > - default_cert, > - default_cert), > - 'wsgi', {'wsgi_app': app}, > - min_threads = 1, > - max_threads = 1) > - server.start() > + server = WSGIServer(("0.0.0.0", 443), app, keyfile = default_cert, > + certfile = default_cert) > + > + server.serve_forever() Indenting? [-- Attachment #2: Type: text/html, Size: 18316 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <201601290049.u0T0nquh003020@d23av02.au.ibm.com>]
* Re: 回复:回复:[PATCH phosphor-rest-server] The streaming support for obmc-rest. [not found] ` <201601290049.u0T0nquh003020@d23av02.au.ibm.com> @ 2016-01-29 2:08 ` Cyril Bur 2016-01-29 3:27 ` Andrew Donnellan 2016-01-29 4:23 ` Peng Fei BG Gou [not found] ` <201601290423.u0T4NYkd013405@d23av02.au.ibm.com> 1 sibling, 2 replies; 8+ messages in thread From: Cyril Bur @ 2016-01-29 2:08 UTC (permalink / raw) To: Peng Fei BG Gou; +Cc: openbmc, OpenBMC Patches On Fri, 29 Jan 2016 00:49:39 +0000 "Peng Fei BG Gou" <shgoupf@cn.ibm.com> wrote: > And the white space you saw in the email thread and coding is probably caused by the mixing use of tab and white space. The original code uses tab for indent, while I prefer to use 4 white spaces for indent. I will discuss with Brad regarding the indent convention for our code. Will fix it up if we strictly need tab in our project. In a world where we all have to read each others code, we should make an effort to not make it hard for everyone. What you have done here is mix different indenting styles which only makes it harder to read for everyone to read the code. Imagine a world where I preferred 8 space indenting and I went through and my additions to the file were 8 space indented, this would be impossible to read. Typically when a file has been written in one way, it is left that way unless that particular way is absolutely abhorrent (5 space indenting...). Tabs are a perfectly valid way of indenting, I don't see the issue here. Futhurmore, I know this isn't really for python, if in doubt we should probably be falling back to the openbmc/docs/contributing file and I quote: Components of the OpenBMC sources should have consistent style. For C code, we typically use the Linux coding style, which is documented at: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/CodingStyle (unless you want to write a completely separate Python document, and change all the python in this project, I suggest we follow this one where appropriate) Indent with tabs instead of spaces, set at 8 columns tl;dr Don't mix indenting styles, way too hard to read the code. > > > > 在 2016年1月29日,上午8:38:15,"Peng Fei BG Gou" <shgoupf@cn.ibm.com> 写道: > > Python is indent based languang, so the function will fail if we have > incorrect indenting. I have tested this in real bmc machine so I believe the > indenting should be fine for now. Please let Brad review this change since he > is familiar with Python. > 在 2016年1月29日,上午8:24:58,"Cyril Bur" <cyrilbur@gmail.com> 写道: > > On Thu, 28 Jan 2016 02:00:37 -0600 > OpenBMC Patches wrote: > > From: shgoupf > > > Hi Peng, > So I'm don't really know python all that well but I do believe this > language is white space sensitive... I'll let a pythoner respond about the > rest... > > 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/ > > --- > > obmc-rest | 115 > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file > > changed, 104 insertions(+), 11 deletions(-) mode change 100644 => > > 100755 obmc-rest > > > > diff --git a/obmc-rest b/obmc-rest > > old mode 100644 > > new mode 100755 > > index c6d2949..481dafa > > --- a/obmc-rest > > +++ b/obmc-rest > > @@ -3,7 +3,9 @@ > > import os > > import sys > > import dbus > > +import gobject > > import dbus.exceptions > > +import dbus.mainloop.glib > > import json > > import logging > > from xml.etree import ElementTree > > @@ -14,6 +16,10 @@ from OpenBMCMapper import Mapper, PathTree, > > IntrospectionNodeParser, ListMatch import spwd > > import grp > > import crypt > > +import threading > > +import gevent > > +from gevent.pywsgi import WSGIServer > > +from gevent.queue import Queue > > > > DBUS_UNKNOWN_INTERFACE = 'org.freedesktop.UnknownInterface' > > DBUS_UNKNOWN_METHOD = 'org.freedesktop.DBus.Error.UnknownMethod' > > @@ -59,12 +65,13 @@ def makelist(data): > > > > class RouteHandler(object): > > _require_auth = makelist(valid_user) > > - def __init__(self, app, bus, verbs, rules): > > + def __init__(self, app, bus, verbs, rules, skips = []): > > self.app = app > > self.bus = bus > > self.mapper = Mapper(bus) > > self._verbs = makelist(verbs) > > self._rules = rules > > + self._skips = skips > > > > def _setup(self, **kw): > > request.route_data = {} > > @@ -79,7 +86,7 @@ class RouteHandler(object): > > return getattr(self, 'do_' + request.method.lower())(**kw) > > > > def install(self): > > - self.app.route(self._rules, callback = self, > > + self.app.route(self._rules, callback = self, skip = self._skips, > > method = ['GET', 'PUT', 'PATCH', 'POST', 'DELETE']) > > > > @staticmethod > > @@ -108,6 +115,58 @@ class RouteHandler(object): > > return None > > raise > > > > +class SignalHandler(RouteHandler): > > + verbs = ['GET'] > > + rules = '/stream/' > > + > > + def __init__(self, app, bus): > > + super(SignalHandler, self).__init__( > > + app, bus, self.verbs, self.rules) > > + > > + def find(self, path, signal): > > + busses = self.try_mapper_call(self.mapper.get_object, > > + path = path) > > + for items in busses.iteritems(): > > + s = self.find_signal_on_bus(path, signal, *items) > > + if s: > > + return s > > + > > + abort(404, _4034_msg %('signal', 'found', signal)) > > + > > + def setup(self, path, signal): > > + request.route_data['map'] = self.find(path, signal) > > + > > + def do_get(self, path, signal): > > + body = Queue() > > + dsignal = DbusSignal(bus, request.route_data['map'][0], > > + request.route_data['map'][1], > > path) > > + dsignal.onData(body.put) > > + dsignal.onFinish(lambda: body.put(StopIteration)) > > + dsignal.signalSnooping() > > + return body > > + > > + @staticmethod > > + def find_signal(signal, signals): > > + if signals is None: > > + return None > > + > > + signal = find_case_insensitive(signal, signals.keys()) > > + if signal is not None: > > + return signal > > + > > + def find_signal_on_bus(self, path, signal, bus, interfaces): > > + obj = self.bus.get_object(bus, path, introspect = False) > > + iface = dbus.Interface(obj, dbus.INTROSPECTABLE_IFACE) > > + data = iface.Introspect() > > + parser = IntrospectionNodeParser( > > + ElementTree.fromstring(data), > > + intf_match = ListMatch(interfaces)) > > + for x,y in parser.get_interfaces().iteritems(): > > + s = self.find_signal(signal, > > + y.get('signal')) > > + if s: > > + return (x,s) > > + > > class DirectoryHandler(RouteHandler): > > verbs = 'GET' > > rules = '/' > > @@ -715,7 +774,8 @@ class RestApp(Bottle): > > self.install(JSONPlugin(**json_kw)) > > self.install(JsonApiErrorsPlugin(**json_kw)) > > self.install(AuthorizationPlugin()) > > - self.install(JsonApiResponsePlugin()) > > + self.json_response_plugin = JsonApiResponsePlugin() > Indenting? > > + self.install(self.json_response_plugin) > > self.install(JsonApiRequestPlugin()) > > self.install(JsonApiRequestTypePlugin()) > > > > @@ -726,6 +786,7 @@ class RestApp(Bottle): > > > > def create_handlers(self): > > # create route handlers > > + self.signal_handler = SignalHandler(self, self.bus) > > self.session_handler = SessionHandler(self, self.bus) > > self.directory_handler = DirectoryHandler(self, self.bus) > > self.list_names_handler = ListNamesHandler(self, self.bus) > > @@ -736,6 +797,11 @@ class RestApp(Bottle): > > self.instance_handler = InstanceHandler(self, self.bus) > > > > def install_handlers(self): > > + # Skip json response for signal handler because it > > requires to > > + # return a gevent iterable which cannot be handled by > > json > > + # response plugin > > + self.signal_handler._skips = > > [self.json_response_plugin] > Indenting? > > + self.signal_handler.install() > > self.session_handler.install() > > self.directory_handler.install() > > self.list_names_handler.install() > > @@ -766,21 +832,48 @@ class RestApp(Bottle): > > parts = filter(bool, path.split('/')) > > request.environ['PATH_INFO'] = '/' + '/'.join(parts) + trailing > > > > +class DbusSignal(): > > + def __init__(self, bus, dbus_interface, signal_name, path): > > + # Register the dbus recieve handler > > + bus.add_signal_receiver(self.signalReciever, > > + dbus_interface = dbus_interface, > > + signal_name = signal_name, > > + path = path) > > + > > + self.snooping = True > > + > > + def signalReciever(self, msg): > > + self.send("Recieved message: %s" % msg) > > + self.snooping = False > > + > > + def onData(self, send): > > + self.send = send > > + > > + def onFinish(self, f): > > + self.finish = f > > + > > + def signalSnooping(self): > > + while self.snooping: > > + mainloop = gobject.MainLoop() > > + gevent.sleep(1) > > + gobject.timeout_add(1, mainloop.quit) > > + mainloop.run() > > + > > + self.finish() > > + > > if __name__ == '__main__': > > log = logging.getLogger('Rocket.Errors') > > log.setLevel(logging.INFO) > > log.addHandler(logging.StreamHandler(sys.stdout)) > > > > + dbus.mainloop.glib.DBusGMainLoop(set_as_default=True) > Indenting? > > bus = dbus.SystemBus() > > app = RestApp(bus) > > + > ? > > default_cert = os.path.join(sys.prefix, 'share', > > os.path.basename(__file__), 'cert.pem') > > > > - server = Rocket(('0.0.0.0', > > - 443, > > - default_cert, > > - default_cert), > > - 'wsgi', {'wsgi_app': app}, > > - min_threads = 1, > > - max_threads = 1) > > - server.start() > > + server = WSGIServer(("0.0.0.0", 443), app, keyfile = > > default_cert, > > + certfile = default_cert) > > + > > + server.serve_forever() > Indenting? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: 回复:回复:[PATCH phosphor-rest-server] The streaming support for obmc-rest. 2016-01-29 2:08 ` Cyril Bur @ 2016-01-29 3:27 ` Andrew Donnellan 2016-01-29 4:23 ` Peng Fei BG Gou 1 sibling, 0 replies; 8+ messages in thread From: Andrew Donnellan @ 2016-01-29 3:27 UTC (permalink / raw) To: openbmc On 29/01/16 13:08, Cyril Bur wrote: > (unless you want to write a completely separate Python document, and change all > the python in this project, I suggest we follow this one where appropriate) > > Indent with tabs instead of spaces, set at 8 columns As someone who has written Python for a living before - PEP8, please! -- Andrew Donnellan Software Engineer, OzLabs andrew.donnellan@au1.ibm.com Australia Development Lab, Canberra +61 2 6201 8874 (work) IBM Australia Limited ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: 回复:回复:[PATCH phosphor-rest-server] The streaming support for obmc-rest. 2016-01-29 2:08 ` Cyril Bur 2016-01-29 3:27 ` Andrew Donnellan @ 2016-01-29 4:23 ` Peng Fei BG Gou 2016-01-29 6:17 ` Vishwanatha Subbanna 1 sibling, 1 reply; 8+ messages in thread From: Peng Fei BG Gou @ 2016-01-29 4:23 UTC (permalink / raw) To: cyrilbur; +Cc: openbmc-patches, openbmc [-- Attachment #1: Type: text/html, Size: 19113 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: 回复:回复:[PATCH phosphor-rest-server] The streaming support for obmc-rest. 2016-01-29 4:23 ` Peng Fei BG Gou @ 2016-01-29 6:17 ` Vishwanatha Subbanna 2016-01-29 6:54 ` Manjunath A Kumatagi 0 siblings, 1 reply; 8+ messages in thread From: Vishwanatha Subbanna @ 2016-01-29 6:17 UTC (permalink / raw) To: Peng Fei BG Gou; +Cc: cyrilbur, openbmc, openbmc-patches [-- Attachment #1.1: Type: text/plain, Size: 13413 bytes --] In the earlier phases of OpenBMC development, I was told to use spaces instead of tabs and I switched to using spaces since then. So all my code is now "4 space" indented ( with VI auto converting my tab to 4 spaces. ) Thanks ------------------------------------------------------------------------------------- Thanks and Regards, Vishwanath. Advisory Software Engineer, Power Firmware Development, Systems &Technology Lab, MG2-6F-255 , Manyata Embassy Business Park, Bangalore , KA , 560045 Ph: +91-80-46678255 E-mail: vishwanath@in.ibm.com ---------------------------------------------------------------------------------- From: "Peng Fei BG Gou" <shgoupf@cn.ibm.com> To: cyrilbur@gmail.com Cc: openbmc@lists.ozlabs.org, openbmc-patches@stwcx.xyz Date: 29/01/2016 09:54 am Subject: Re: 回复:回复:[PATCH phosphor-rest-server] The streaming support for obmc-rest. Sent by: "openbmc" <openbmc-bounces +vishwanath=in.ibm.com@lists.ozlabs.org> Yes Cyril, your suggestion is fair, we should keep it consistent across all files in a project. There is always debating between tabs and whitespaces. Even though personally I prefer white spaces (that's why my editor automatically convert tab to whitespace for me), I agree that for this project, I should use tabs. Will make an update soon. GOU, Peng Fei (苟鹏飞), Ph.D. OpenPOWER Enablement. +86-21-609-28631 ----- Original message ----- From: Cyril Bur <cyrilbur@gmail.com> To: Peng Fei BG Gou/China/IBM@IBMCN Cc: "openbmc" <openbmc@lists.ozlabs.org>, "OpenBMC Patches" <openbmc-patches@stwcx.xyz> Subject: Re: 回复:回复:[PATCH phosphor-rest-server] The streaming support for obmc-rest. Date: Fri, Jan 29, 2016 10:09 AM On Fri, 29 Jan 2016 00:49:39 +0000 "Peng Fei BG Gou" <shgoupf@cn.ibm.com> wrote: > And the white space you saw in the email thread and coding is probably caused by the mixing use of tab and white space. The original code uses tab for indent, while I prefer to use 4 white spaces for indent. I will discuss with Brad regarding the indent convention for our code. Will fix it up if we strictly need tab in our project. In a world where we all have to read each others code, we should make an effort to not make it hard for everyone. What you have done here is mix different indenting styles which only makes it harder to read for everyone to read the code. Imagine a world where I preferred 8 space indenting and I went through and my additions to the file were 8 space indented, this would be impossible to read. Typically when a file has been written in one way, it is left that way unless that particular way is absolutely abhorrent (5 space indenting...). Tabs are a perfectly valid way of indenting, I don't see the issue here. Futhurmore, I know this isn't really for python, if in doubt we should probably be falling back to the openbmc/docs/contributing file and I quote: Components of the OpenBMC sources should have consistent style. For C code, we typically use the Linux coding style, which is documented at: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/CodingStyle (unless you want to write a completely separate Python document, and change all the python in this project, I suggest we follow this one where appropriate) Indent with tabs instead of spaces, set at 8 columns tl;dr Don't mix indenting styles, way too hard to read the code. > > > > 在 2016年1月29日,上午8:38:15,"Peng Fei BG Gou" <shgoupf@cn.ibm.com> 写道: > > Python is indent based languang, so the function will fail if we have > incorrect indenting. I have tested this in real bmc machine so I believe the > indenting should be fine for now. Please let Brad review this change since he > is familiar with Python. > 在 2016年1月29日,上午8:24:58,"Cyril Bur" <cyrilbur@gmail.com> 写 道: > > On Thu, 28 Jan 2016 02:00:37 -0600 > OpenBMC Patches wrote: > > From: shgoupf > > > Hi Peng, > So I'm don't really know python all that well but I do believe this > language is white space sensitive... I'll let a pythoner respond about the > rest... > > 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/ > > --- > > obmc-rest | 115 > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file > > changed, 104 insertions(+), 11 deletions(-) mode change 100644 => > > 100755 obmc-rest > > > > diff --git a/obmc-rest b/obmc-rest > > old mode 100644 > > new mode 100755 > > index c6d2949..481dafa > > --- a/obmc-rest > > +++ b/obmc-rest > > @@ -3,7 +3,9 @@ > > import os > > import sys > > import dbus > > +import gobject > > import dbus.exceptions > > +import dbus.mainloop.glib > > import json > > import logging > > from xml.etree import ElementTree > > @@ -14,6 +16,10 @@ from OpenBMCMapper import Mapper, PathTree, > > IntrospectionNodeParser, ListMatch import spwd > > import grp > > import crypt > > +import threading > > +import gevent > > +from gevent.pywsgi import WSGIServer > > +from gevent.queue import Queue > > > > DBUS_UNKNOWN_INTERFACE = 'org.freedesktop.UnknownInterface' > > DBUS_UNKNOWN_METHOD = 'org.freedesktop.DBus.Error.UnknownMethod' > > @@ -59,12 +65,13 @@ def makelist(data): > > > > class RouteHandler(object): > > _require_auth = makelist(valid_user) > > - def __init__(self, app, bus, verbs, rules): > > + def __init__(self, app, bus, verbs, rules, skips = []): > > self.app = app > > self.bus = bus > > self.mapper = Mapper(bus) > > self._verbs = makelist(verbs) > > self._rules = rules > > + self._skips = skips > > > > def _setup(self, **kw): > > request.route_data = {} > > @@ -79,7 +86,7 @@ class RouteHandler(object): > > return getattr(self, 'do_' + request.method.lower())(**kw) > > > > def install(self): > > - self.app.route(self._rules, callback = self, > > + self.app.route(self._rules, callback = self, skip = self._skips, > > method = ['GET', 'PUT', 'PATCH', 'POST', 'DELETE']) > > > > @staticmethod > > @@ -108,6 +115,58 @@ class RouteHandler(object): > > return None > > raise > > > > +class SignalHandler(RouteHandler): > > + verbs = ['GET'] > > + rules = '/stream/' > > + > > + def __init__(self, app, bus): > > + super(SignalHandler, self).__init__( > > + app, bus, self.verbs, self.rules) > > + > > + def find(self, path, signal): > > + busses = self.try_mapper_call(self.mapper.get_object, > > + path = path) > > + for items in busses.iteritems(): > > + s = self.find_signal_on_bus(path, signal, *items) > > + if s: > > + return s > > + > > + abort(404, _4034_msg %('signal', 'found', signal)) > > + > > + def setup(self, path, signal): > > + request.route_data['map'] = self.find(path, signal) > > + > > + def do_get(self, path, signal): > > + body = Queue() > > + dsignal = DbusSignal(bus, request.route_data ['map'][0], > > + request.route_data ['map'][1], > > path) > > + dsignal.onData(body.put) > > + dsignal.onFinish(lambda: body.put(StopIteration)) > > + dsignal.signalSnooping() > > + return body > > + > > + @staticmethod > > + def find_signal(signal, signals): > > + if signals is None: > > + return None > > + > > + signal = find_case_insensitive(signal, signals.keys()) > > + if signal is not None: > > + return signal > > + > > + def find_signal_on_bus(self, path, signal, bus, interfaces): > > + obj = self.bus.get_object(bus, path, introspect = False) > > + iface = dbus.Interface(obj, dbus.INTROSPECTABLE_IFACE) > > + data = iface.Introspect() > > + parser = IntrospectionNodeParser( > > + ElementTree.fromstring(data), > > + intf_match = ListMatch(interfaces)) > > + for x,y in parser.get_interfaces().iteritems(): > > + s = self.find_signal(signal, > > + y.get('signal')) > > + if s: > > + return (x,s) > > + > > class DirectoryHandler(RouteHandler): > > verbs = 'GET' > > rules = '/' > > @@ -715,7 +774,8 @@ class RestApp(Bottle): > > self.install(JSONPlugin(**json_kw)) > > self.install(JsonApiErrorsPlugin(**json_kw)) > > self.install(AuthorizationPlugin()) > > - self.install(JsonApiResponsePlugin()) > > + self.json_response_plugin = JsonApiResponsePlugin () > Indenting? > > + self.install(self.json_response_plugin) > > self.install(JsonApiRequestPlugin()) > > self.install(JsonApiRequestTypePlugin()) > > > > @@ -726,6 +786,7 @@ class RestApp(Bottle): > > > > def create_handlers(self): > > # create route handlers > > + self.signal_handler = SignalHandler(self, self.bus) > > self.session_handler = SessionHandler(self, self.bus) > > self.directory_handler = DirectoryHandler(self, self.bus) > > self.list_names_handler = ListNamesHandler(self, self.bus) > > @@ -736,6 +797,11 @@ class RestApp(Bottle): > > self.instance_handler = InstanceHandler(self, self.bus) > > > > def install_handlers(self): > > + # Skip json response for signal handler because it > > requires to > > + # return a gevent iterable which cannot be handled by > > json > > + # response plugin > > + self.signal_handler._skips = > > [self.json_response_plugin] > Indenting? > > + self.signal_handler.install() > > self.session_handler.install() > > self.directory_handler.install() > > self.list_names_handler.install() > > @@ -766,21 +832,48 @@ class RestApp(Bottle): > > parts = filter(bool, path.split('/')) > > request.environ['PATH_INFO'] = '/' + '/'.join(parts) + trailing > > > > +class DbusSignal(): > > + def __init__(self, bus, dbus_interface, signal_name, path): > > + # Register the dbus recieve handler > > + bus.add_signal_receiver(self.signalReciever, > > + dbus_interface = dbus_interface, > > + signal_name = signal_name, > > + path = path) > > + > > + self.snooping = True > > + > > + def signalReciever(self, msg): > > + self.send("Recieved message: %s" % msg) > > + self.snooping = False > > + > > + def onData(self, send): > > + self.send = send > > + > > + def onFinish(self, f): > > + self.finish = f > > + > > + def signalSnooping(self): > > + while self.snooping: > > + mainloop = gobject.MainLoop() > > + gevent.sleep(1) > > + gobject.timeout_add(1, mainloop.quit) > > + mainloop.run() > > + > > + self.finish() > > + > > if __name__ == '__main__': > > log = logging.getLogger('Rocket.Errors') > > log.setLevel(logging.INFO) > > log.addHandler(logging.StreamHandler(sys.stdout)) > > > > + dbus.mainloop.glib.DBusGMainLoop(set_as_default=True) > Indenting? > > bus = dbus.SystemBus() > > app = RestApp(bus) > > + > ? > > default_cert = os.path.join(sys.prefix, 'share', > > os.path.basename(__file__), 'cert.pem') > > > > - server = Rocket(('0.0.0.0', > > - 443, > > - default_cert, > > - default_cert), > > - 'wsgi', {'wsgi_app': app}, > > - min_threads = 1, > > - max_threads = 1) > > - server.start() > > + server = WSGIServer(("0.0.0.0", 443), app, keyfile = > > default_cert, > > + certfile = default_cert) > > + > > + server.serve_forever() > Indenting? _______________________________________________ openbmc mailing list openbmc@lists.ozlabs.org https://lists.ozlabs.org/listinfo/openbmc [-- Attachment #1.2: Type: text/html, Size: 21064 bytes --] [-- Attachment #2: graycol.gif --] [-- Type: image/gif, Size: 105 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: 回复:回复:[PATCH phosphor-rest-server] The streaming support for obmc-rest. 2016-01-29 6:17 ` Vishwanatha Subbanna @ 2016-01-29 6:54 ` Manjunath A Kumatagi 2016-01-29 6:57 ` Manjunath A Kumatagi 0 siblings, 1 reply; 8+ messages in thread From: Manjunath A Kumatagi @ 2016-01-29 6:54 UTC (permalink / raw) To: Vishwanatha Subbanna; +Cc: Peng Fei BG Gou, openbmc, openbmc-patches, cyrilbur [-- Attachment #1.1: Type: text/plain, Size: 14221 bytes --] Yes, you are right Viswa! PEP8 recommends to have spaces(in multiples of 4), here is the link for PEP8 standards which talks about the standards need to follow for the code. Going forward it is really nice idea to use pep8 tool to check the coding standard for python code. Thanks, Manjunath. From: Vishwanatha Subbanna/India/IBM@IBMIN To: "Peng Fei BG Gou" <shgoupf@cn.ibm.com> Cc: openbmc@lists.ozlabs.org, openbmc-patches@stwcx.xyz, cyrilbur@gmail.com Date: 01/29/2016 11:47 AM Subject: Re: 回复:回复:[PATCH phosphor-rest-server] The streaming support for obmc-rest. Sent by: "openbmc" <openbmc-bounces +mkumatag=in.ibm.com@lists.ozlabs.org> In the earlier phases of OpenBMC development, I was told to use spaces instead of tabs and I switched to using spaces since then. So all my code is now "4 space" indented ( with VI auto converting my tab to 4 spaces. ) Thanks ------------------------------------------------------------------------------------- Thanks and Regards, Vishwanath. Advisory Software Engineer, Power Firmware Development, Systems &Technology Lab, MG2-6F-255 , Manyata Embassy Business Park, Bangalore , KA , 560045 Ph: +91-80-46678255 E-mail: vishwanath@in.ibm.com ---------------------------------------------------------------------------------- Inactive hide details for "Peng Fei BG Gou" ---29/01/2016 09:54:28 am---Yes Cyril, your suggestion is fair, we should keep it c"Peng Fei BG Gou" ---29/01/2016 09:54:28 am---Yes Cyril, your suggestion is fair, we should keep it consistent across all files in a project. Ther From: "Peng Fei BG Gou" <shgoupf@cn.ibm.com> To: cyrilbur@gmail.com Cc: openbmc@lists.ozlabs.org, openbmc-patches@stwcx.xyz Date: 29/01/2016 09:54 am Subject: Re: 回复:回复:[PATCH phosphor-rest-server] The streaming support for obmc-rest. Sent by: "openbmc" <openbmc-bounces+vishwanath=in.ibm.com@lists.ozlabs.org> Yes Cyril, your suggestion is fair, we should keep it consistent across all files in a project. There is always debating between tabs and whitespaces. Even though personally I prefer white spaces (that's why my editor automatically convert tab to whitespace for me), I agree that for this project, I should use tabs. Will make an update soon. GOU, Peng Fei (苟鹏飞), Ph.D. OpenPOWER Enablement. +86-21-609-28631 ----- Original message ----- From: Cyril Bur <cyrilbur@gmail.com> To: Peng Fei BG Gou/China/IBM@IBMCN Cc: "openbmc" <openbmc@lists.ozlabs.org>, "OpenBMC Patches" <openbmc-patches@stwcx.xyz> Subject: Re: 回复:回复:[PATCH phosphor-rest-server] The streaming support for obmc-rest. Date: Fri, Jan 29, 2016 10:09 AM On Fri, 29 Jan 2016 00:49:39 +0000 "Peng Fei BG Gou" <shgoupf@cn.ibm.com> wrote: > And the white space you saw in the email thread and coding is probably caused by the mixing use of tab and white space. The original code uses tab for indent, while I prefer to use 4 white spaces for indent. I will discuss with Brad regarding the indent convention for our code. Will fix it up if we strictly need tab in our project. In a world where we all have to read each others code, we should make an effort to not make it hard for everyone. What you have done here is mix different indenting styles which only makes it harder to read for everyone to read the code. Imagine a world where I preferred 8 space indenting and I went through and my additions to the file were 8 space indented, this would be impossible to read. Typically when a file has been written in one way, it is left that way unless that particular way is absolutely abhorrent (5 space indenting...). Tabs are a perfectly valid way of indenting, I don't see the issue here. Futhurmore, I know this isn't really for python, if in doubt we should probably be falling back to the openbmc/docs/contributing file and I quote: Components of the OpenBMC sources should have consistent style. For C code, we typically use the Linux coding style, which is documented at: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/CodingStyle (unless you want to write a completely separate Python document, and change all the python in this project, I suggest we follow this one where appropriate) Indent with tabs instead of spaces, set at 8 columns tl;dr Don't mix indenting styles, way too hard to read the code. > > > > 在 2016年1月29日,上午8:38:15,"Peng Fei BG Gou" <shgoupf@cn.ibm.com> 写 道: > > Python is indent based languang, so the function will fail if we have > incorrect indenting. I have tested this in real bmc machine so I believe the > indenting should be fine for now. Please let Brad review this change since he > is familiar with Python. > 在 2016年1月29日,上午8:24:58,"Cyril Bur" <cyrilbur@gmail.com> 写道: > > On Thu, 28 Jan 2016 02:00:37 -0600 > OpenBMC Patches wrote: > > From: shgoupf > > > Hi Peng, > So I'm don't really know python all that well but I do believe this > language is white space sensitive... I'll let a pythoner respond about the > rest... > > 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/ > > --- > > obmc-rest | 115 > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file > > changed, 104 insertions(+), 11 deletions(-) mode change 100644 => > > 100755 obmc-rest > > > > diff --git a/obmc-rest b/obmc-rest > > old mode 100644 > > new mode 100755 > > index c6d2949..481dafa > > --- a/obmc-rest > > +++ b/obmc-rest > > @@ -3,7 +3,9 @@ > > import os > > import sys > > import dbus > > +import gobject > > import dbus.exceptions > > +import dbus.mainloop.glib > > import json > > import logging > > from xml.etree import ElementTree > > @@ -14,6 +16,10 @@ from OpenBMCMapper import Mapper, PathTree, > > IntrospectionNodeParser, ListMatch import spwd > > import grp > > import crypt > > +import threading > > +import gevent > > +from gevent.pywsgi import WSGIServer > > +from gevent.queue import Queue > > > > DBUS_UNKNOWN_INTERFACE = 'org.freedesktop.UnknownInterface' > > DBUS_UNKNOWN_METHOD = 'org.freedesktop.DBus.Error.UnknownMethod' > > @@ -59,12 +65,13 @@ def makelist(data): > > > > class RouteHandler(object): > > _require_auth = makelist(valid_user) > > - def __init__(self, app, bus, verbs, rules): > > + def __init__(self, app, bus, verbs, rules, skips = []): > > self.app = app > > self.bus = bus > > self.mapper = Mapper(bus) > > self._verbs = makelist(verbs) > > self._rules = rules > > + self._skips = skips > > > > def _setup(self, **kw): > > request.route_data = {} > > @@ -79,7 +86,7 @@ class RouteHandler(object): > > return getattr(self, 'do_' + request.method.lower())(**kw) > > > > def install(self): > > - self.app.route(self._rules, callback = self, > > + self.app.route(self._rules, callback = self, skip = self._skips, > > method = ['GET', 'PUT', 'PATCH', 'POST', 'DELETE']) > > > > @staticmethod > > @@ -108,6 +115,58 @@ class RouteHandler(object): > > return None > > raise > > > > +class SignalHandler(RouteHandler): > > + verbs = ['GET'] > > + rules = '/stream/' > > + > > + def __init__(self, app, bus): > > + super(SignalHandler, self).__init__( > > + app, bus, self.verbs, self.rules) > > + > > + def find(self, path, signal): > > + busses = self.try_mapper_call(self.mapper.get_object, > > + path = path) > > + for items in busses.iteritems(): > > + s = self.find_signal_on_bus(path, signal, *items) > > + if s: > > + return s > > + > > + abort(404, _4034_msg %('signal', 'found', signal)) > > + > > + def setup(self, path, signal): > > + request.route_data['map'] = self.find(path, signal) > > + > > + def do_get(self, path, signal): > > + body = Queue() > > + dsignal = DbusSignal(bus, request.route_data ['map'][0], > > + request.route_data['map'][1], > > path) > > + dsignal.onData(body.put) > > + dsignal.onFinish(lambda: body.put(StopIteration)) > > + dsignal.signalSnooping() > > + return body > > + > > + @staticmethod > > + def find_signal(signal, signals): > > + if signals is None: > > + return None > > + > > + signal = find_case_insensitive(signal, signals.keys()) > > + if signal is not None: > > + return signal > > + > > + def find_signal_on_bus(self, path, signal, bus, interfaces): > > + obj = self.bus.get_object(bus, path, introspect = False) > > + iface = dbus.Interface(obj, dbus.INTROSPECTABLE_IFACE) > > + data = iface.Introspect() > > + parser = IntrospectionNodeParser( > > + ElementTree.fromstring(data), > > + intf_match = ListMatch(interfaces)) > > + for x,y in parser.get_interfaces().iteritems(): > > + s = self.find_signal(signal, > > + y.get('signal')) > > + if s: > > + return (x,s) > > + > > class DirectoryHandler(RouteHandler): > > verbs = 'GET' > > rules = '/' > > @@ -715,7 +774,8 @@ class RestApp(Bottle): > > self.install(JSONPlugin(**json_kw)) > > self.install(JsonApiErrorsPlugin(**json_kw)) > > self.install(AuthorizationPlugin()) > > - self.install(JsonApiResponsePlugin()) > > + self.json_response_plugin = JsonApiResponsePlugin () > Indenting? > > + self.install(self.json_response_plugin) > > self.install(JsonApiRequestPlugin()) > > self.install(JsonApiRequestTypePlugin()) > > > > @@ -726,6 +786,7 @@ class RestApp(Bottle): > > > > def create_handlers(self): > > # create route handlers > > + self.signal_handler = SignalHandler(self, self.bus) > > self.session_handler = SessionHandler(self, self.bus) > > self.directory_handler = DirectoryHandler(self, self.bus) > > self.list_names_handler = ListNamesHandler(self, self.bus) > > @@ -736,6 +797,11 @@ class RestApp(Bottle): > > self.instance_handler = InstanceHandler(self, self.bus) > > > > def install_handlers(self): > > + # Skip json response for signal handler because it > > requires to > > + # return a gevent iterable which cannot be handled by > > json > > + # response plugin > > + self.signal_handler._skips = > > [self.json_response_plugin] > Indenting? > > + self.signal_handler.install() > > self.session_handler.install() > > self.directory_handler.install() > > self.list_names_handler.install() > > @@ -766,21 +832,48 @@ class RestApp(Bottle): > > parts = filter(bool, path.split('/')) > > request.environ['PATH_INFO'] = '/' + '/'.join(parts) + trailing > > > > +class DbusSignal(): > > + def __init__(self, bus, dbus_interface, signal_name, path): > > + # Register the dbus recieve handler > > + bus.add_signal_receiver(self.signalReciever, > > + dbus_interface = dbus_interface, > > + signal_name = signal_name, > > + path = path) > > + > > + self.snooping = True > > + > > + def signalReciever(self, msg): > > + self.send("Recieved message: %s" % msg) > > + self.snooping = False > > + > > + def onData(self, send): > > + self.send = send > > + > > + def onFinish(self, f): > > + self.finish = f > > + > > + def signalSnooping(self): > > + while self.snooping: > > + mainloop = gobject.MainLoop() > > + gevent.sleep(1) > > + gobject.timeout_add(1, mainloop.quit) > > + mainloop.run() > > + > > + self.finish() > > + > > if __name__ == '__main__': > > log = logging.getLogger('Rocket.Errors') > > log.setLevel(logging.INFO) > > log.addHandler(logging.StreamHandler(sys.stdout)) > > > > + dbus.mainloop.glib.DBusGMainLoop(set_as_default=True) > Indenting? > > bus = dbus.SystemBus() > > app = RestApp(bus) > > + > ? > > default_cert = os.path.join(sys.prefix, 'share', > > os.path.basename(__file__), 'cert.pem') > > > > - server = Rocket(('0.0.0.0', > > - 443, > > - default_cert, > > - default_cert), > > - 'wsgi', {'wsgi_app': app}, > > - min_threads = 1, > > - max_threads = 1) > > - server.start() > > + server = WSGIServer(("0.0.0.0", 443), app, keyfile = > > default_cert, > > + certfile = default_cert) > > + > > + server.serve_forever() > Indenting? _______________________________________________ openbmc mailing list openbmc@lists.ozlabs.org https://lists.ozlabs.org/listinfo/openbmc _______________________________________________ openbmc mailing list openbmc@lists.ozlabs.org https://lists.ozlabs.org/listinfo/openbmc [-- Attachment #1.2: Type: text/html, Size: 23001 bytes --] [-- Attachment #2: graycol.gif --] [-- Type: image/gif, Size: 105 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: 回复:回复:[PATCH phosphor-rest-server] The streaming support for obmc-rest. 2016-01-29 6:54 ` Manjunath A Kumatagi @ 2016-01-29 6:57 ` Manjunath A Kumatagi 0 siblings, 0 replies; 8+ messages in thread From: Manjunath A Kumatagi @ 2016-01-29 6:57 UTC (permalink / raw) To: Manjunath A Kumatagi Cc: Vishwanatha Subbanna, openbmc, openbmc-patches, cyrilbur, Peng Fei BG Gou [-- Attachment #1.1: Type: text/plain, Size: 15110 bytes --] link for PEP8 - https://www.python.org/dev/peps/pep-0008/ From: Manjunath A Kumatagi/India/IBM@IBMIN To: Vishwanatha Subbanna/India/IBM@IBMIN Cc: openbmc@lists.ozlabs.org, openbmc-patches@stwcx.xyz, cyrilbur@gmail.com, Peng Fei BG Gou <shgoupf@cn.ibm.com> Date: 01/29/2016 12:25 PM Subject: Re: 回复:回复:[PATCH phosphor-rest-server] The streaming support for obmc-rest. Sent by: "openbmc" <openbmc-bounces +mkumatag=in.ibm.com@lists.ozlabs.org> Yes, you are right Viswa! PEP8 recommends to have spaces(in multiples of 4), here is the link for PEP8 standards which talks about the standards need to follow for the code. Going forward it is really nice idea to use pep8 tool to check the coding standard for python code. Thanks, Manjunath. Inactive hide details for Vishwanatha Subbanna---01/29/2016 11:47:53 AM---In the earlier phases of OpenBMC development, I was tVishwanatha Subbanna---01/29/2016 11:47:53 AM---In the earlier phases of OpenBMC development, I was told to use spaces instead of tabs and I switche From: Vishwanatha Subbanna/India/IBM@IBMIN To: "Peng Fei BG Gou" <shgoupf@cn.ibm.com> Cc: openbmc@lists.ozlabs.org, openbmc-patches@stwcx.xyz, cyrilbur@gmail.com Date: 01/29/2016 11:47 AM Subject: Re: 回复:回复:[PATCH phosphor-rest-server] The streaming support for obmc-rest. Sent by: "openbmc" <openbmc-bounces+mkumatag=in.ibm.com@lists.ozlabs.org> In the earlier phases of OpenBMC development, I was told to use spaces instead of tabs and I switched to using spaces since then. So all my code is now "4 space" indented ( with VI auto converting my tab to 4 spaces. ) Thanks ------------------------------------------------------------------------------------- Thanks and Regards, Vishwanath. Advisory Software Engineer, Power Firmware Development, Systems &Technology Lab, MG2-6F-255 , Manyata Embassy Business Park, Bangalore , KA , 560045 Ph: +91-80-46678255 E-mail: vishwanath@in.ibm.com ---------------------------------------------------------------------------------- Inactive hide details for "Peng Fei BG Gou" ---29/01/2016 09:54:28 am---Yes Cyril, your suggestion is fair, we should keep it c"Peng Fei BG Gou" ---29/01/2016 09:54:28 am---Yes Cyril, your suggestion is fair, we should keep it consistent across all files in a project. Ther From: "Peng Fei BG Gou" <shgoupf@cn.ibm.com> To: cyrilbur@gmail.com Cc: openbmc@lists.ozlabs.org, openbmc-patches@stwcx.xyz Date: 29/01/2016 09:54 am Subject: Re: 回复:回复:[PATCH phosphor-rest-server] The streaming support for obmc-rest. Sent by: "openbmc" <openbmc-bounces+vishwanath=in.ibm.com@lists.ozlabs.org> Yes Cyril, your suggestion is fair, we should keep it consistent across all files in a project. There is always debating between tabs and whitespaces. Even though personally I prefer white spaces (that's why my editor automatically convert tab to whitespace for me), I agree that for this project, I should use tabs. Will make an update soon. GOU, Peng Fei (苟鹏飞), Ph.D. OpenPOWER Enablement. +86-21-609-28631 ----- Original message ----- From: Cyril Bur <cyrilbur@gmail.com> To: Peng Fei BG Gou/China/IBM@IBMCN Cc: "openbmc" <openbmc@lists.ozlabs.org>, "OpenBMC Patches" <openbmc-patches@stwcx.xyz> Subject: Re: 回复:回复:[PATCH phosphor-rest-server] The streaming support for obmc-rest. Date: Fri, Jan 29, 2016 10:09 AM On Fri, 29 Jan 2016 00:49:39 +0000 "Peng Fei BG Gou" <shgoupf@cn.ibm.com> wrote: > And the white space you saw in the email thread and coding is probably caused by the mixing use of tab and white space. The original code uses tab for indent, while I prefer to use 4 white spaces for indent. I will discuss with Brad regarding the indent convention for our code. Will fix it up if we strictly need tab in our project. In a world where we all have to read each others code, we should make an effort to not make it hard for everyone. What you have done here is mix different indenting styles which only makes it harder to read for everyone to read the code. Imagine a world where I preferred 8 space indenting and I went through and my additions to the file were 8 space indented, this would be impossible to read. Typically when a file has been written in one way, it is left that way unless that particular way is absolutely abhorrent (5 space indenting...). Tabs are a perfectly valid way of indenting, I don't see the issue here. Futhurmore, I know this isn't really for python, if in doubt we should probably be falling back to the openbmc/docs/contributing file and I quote: Components of the OpenBMC sources should have consistent style. For C code, we typically use the Linux coding style, which is documented at: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/CodingStyle (unless you want to write a completely separate Python document, and change all the python in this project, I suggest we follow this one where appropriate) Indent with tabs instead of spaces, set at 8 columns tl;dr Don't mix indenting styles, way too hard to read the code. > > > > 在 2016年1月29日,上午8:38:15,"Peng Fei BG Gou" <shgoupf@cn.ibm.com> 写 道: > > Python is indent based languang, so the function will fail if we have > incorrect indenting. I have tested this in real bmc machine so I believe the > indenting should be fine for now. Please let Brad review this change since he > is familiar with Python. > 在 2016年1月29日,上午8:24:58,"Cyril Bur" <cyrilbur@gmail.com> 写道: > > On Thu, 28 Jan 2016 02:00:37 -0600 > OpenBMC Patches wrote: > > From: shgoupf > > > Hi Peng, > So I'm don't really know python all that well but I do believe this > language is white space sensitive... I'll let a pythoner respond about the > rest... > > 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/ > > --- > > obmc-rest | 115 > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file > > changed, 104 insertions(+), 11 deletions(-) mode change 100644 => > > 100755 obmc-rest > > > > diff --git a/obmc-rest b/obmc-rest > > old mode 100644 > > new mode 100755 > > index c6d2949..481dafa > > --- a/obmc-rest > > +++ b/obmc-rest > > @@ -3,7 +3,9 @@ > > import os > > import sys > > import dbus > > +import gobject > > import dbus.exceptions > > +import dbus.mainloop.glib > > import json > > import logging > > from xml.etree import ElementTree > > @@ -14,6 +16,10 @@ from OpenBMCMapper import Mapper, PathTree, > > IntrospectionNodeParser, ListMatch import spwd > > import grp > > import crypt > > +import threading > > +import gevent > > +from gevent.pywsgi import WSGIServer > > +from gevent.queue import Queue > > > > DBUS_UNKNOWN_INTERFACE = 'org.freedesktop.UnknownInterface' > > DBUS_UNKNOWN_METHOD = 'org.freedesktop.DBus.Error.UnknownMethod' > > @@ -59,12 +65,13 @@ def makelist(data): > > > > class RouteHandler(object): > > _require_auth = makelist(valid_user) > > - def __init__(self, app, bus, verbs, rules): > > + def __init__(self, app, bus, verbs, rules, skips = []): > > self.app = app > > self.bus = bus > > self.mapper = Mapper(bus) > > self._verbs = makelist(verbs) > > self._rules = rules > > + self._skips = skips > > > > def _setup(self, **kw): > > request.route_data = {} > > @@ -79,7 +86,7 @@ class RouteHandler(object): > > return getattr(self, 'do_' + request.method.lower())(**kw) > > > > def install(self): > > - self.app.route(self._rules, callback = self, > > + self.app.route(self._rules, callback = self, skip = self._skips, > > method = ['GET', 'PUT', 'PATCH', 'POST', 'DELETE']) > > > > @staticmethod > > @@ -108,6 +115,58 @@ class RouteHandler(object): > > return None > > raise > > > > +class SignalHandler(RouteHandler): > > + verbs = ['GET'] > > + rules = '/stream/' > > + > > + def __init__(self, app, bus): > > + super(SignalHandler, self).__init__( > > + app, bus, self.verbs, self.rules) > > + > > + def find(self, path, signal): > > + busses = self.try_mapper_call(self.mapper.get_object, > > + path = path) > > + for items in busses.iteritems(): > > + s = self.find_signal_on_bus(path, signal, *items) > > + if s: > > + return s > > + > > + abort(404, _4034_msg %('signal', 'found', signal)) > > + > > + def setup(self, path, signal): > > + request.route_data['map'] = self.find(path, signal) > > + > > + def do_get(self, path, signal): > > + body = Queue() > > + dsignal = DbusSignal(bus, request.route_data ['map'][0], > > + request.route_data['map'][1], > > path) > > + dsignal.onData(body.put) > > + dsignal.onFinish(lambda: body.put(StopIteration)) > > + dsignal.signalSnooping() > > + return body > > + > > + @staticmethod > > + def find_signal(signal, signals): > > + if signals is None: > > + return None > > + > > + signal = find_case_insensitive(signal, signals.keys()) > > + if signal is not None: > > + return signal > > + > > + def find_signal_on_bus(self, path, signal, bus, interfaces): > > + obj = self.bus.get_object(bus, path, introspect = False) > > + iface = dbus.Interface(obj, dbus.INTROSPECTABLE_IFACE) > > + data = iface.Introspect() > > + parser = IntrospectionNodeParser( > > + ElementTree.fromstring(data), > > + intf_match = ListMatch(interfaces)) > > + for x,y in parser.get_interfaces().iteritems(): > > + s = self.find_signal(signal, > > + y.get('signal')) > > + if s: > > + return (x,s) > > + > > class DirectoryHandler(RouteHandler): > > verbs = 'GET' > > rules = '/' > > @@ -715,7 +774,8 @@ class RestApp(Bottle): > > self.install(JSONPlugin(**json_kw)) > > self.install(JsonApiErrorsPlugin(**json_kw)) > > self.install(AuthorizationPlugin()) > > - self.install(JsonApiResponsePlugin()) > > + self.json_response_plugin = JsonApiResponsePlugin () > Indenting? > > + self.install(self.json_response_plugin) > > self.install(JsonApiRequestPlugin()) > > self.install(JsonApiRequestTypePlugin()) > > > > @@ -726,6 +786,7 @@ class RestApp(Bottle): > > > > def create_handlers(self): > > # create route handlers > > + self.signal_handler = SignalHandler(self, self.bus) > > self.session_handler = SessionHandler(self, self.bus) > > self.directory_handler = DirectoryHandler(self, self.bus) > > self.list_names_handler = ListNamesHandler(self, self.bus) > > @@ -736,6 +797,11 @@ class RestApp(Bottle): > > self.instance_handler = InstanceHandler(self, self.bus) > > > > def install_handlers(self): > > + # Skip json response for signal handler because it > > requires to > > + # return a gevent iterable which cannot be handled by > > json > > + # response plugin > > + self.signal_handler._skips = > > [self.json_response_plugin] > Indenting? > > + self.signal_handler.install() > > self.session_handler.install() > > self.directory_handler.install() > > self.list_names_handler.install() > > @@ -766,21 +832,48 @@ class RestApp(Bottle): > > parts = filter(bool, path.split('/')) > > request.environ['PATH_INFO'] = '/' + '/'.join(parts) + trailing > > > > +class DbusSignal(): > > + def __init__(self, bus, dbus_interface, signal_name, path): > > + # Register the dbus recieve handler > > + bus.add_signal_receiver(self.signalReciever, > > + dbus_interface = dbus_interface, > > + signal_name = signal_name, > > + path = path) > > + > > + self.snooping = True > > + > > + def signalReciever(self, msg): > > + self.send("Recieved message: %s" % msg) > > + self.snooping = False > > + > > + def onData(self, send): > > + self.send = send > > + > > + def onFinish(self, f): > > + self.finish = f > > + > > + def signalSnooping(self): > > + while self.snooping: > > + mainloop = gobject.MainLoop() > > + gevent.sleep(1) > > + gobject.timeout_add(1, mainloop.quit) > > + mainloop.run() > > + > > + self.finish() > > + > > if __name__ == '__main__': > > log = logging.getLogger('Rocket.Errors') > > log.setLevel(logging.INFO) > > log.addHandler(logging.StreamHandler(sys.stdout)) > > > > + dbus.mainloop.glib.DBusGMainLoop(set_as_default=True) > Indenting? > > bus = dbus.SystemBus() > > app = RestApp(bus) > > + > ? > > default_cert = os.path.join(sys.prefix, 'share', > > os.path.basename(__file__), 'cert.pem') > > > > - server = Rocket(('0.0.0.0', > > - 443, > > - default_cert, > > - default_cert), > > - 'wsgi', {'wsgi_app': app}, > > - min_threads = 1, > > - max_threads = 1) > > - server.start() > > + server = WSGIServer(("0.0.0.0", 443), app, keyfile = > > default_cert, > > + certfile = default_cert) > > + > > + server.serve_forever() > Indenting? _______________________________________________ openbmc mailing list openbmc@lists.ozlabs.org https://lists.ozlabs.org/listinfo/openbmc _______________________________________________ openbmc mailing list openbmc@lists.ozlabs.org https://lists.ozlabs.org/listinfo/openbmc _______________________________________________ openbmc mailing list openbmc@lists.ozlabs.org https://lists.ozlabs.org/listinfo/openbmc [-- Attachment #1.2: Type: text/html, Size: 24790 bytes --] [-- Attachment #2: graycol.gif --] [-- Type: image/gif, Size: 105 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <201601290423.u0T4NYkd013405@d23av02.au.ibm.com>]
* Re: 回复:回复:[PATCH phosphor-rest-server] The streaming support for obmc-rest. [not found] ` <201601290423.u0T4NYkd013405@d23av02.au.ibm.com> @ 2016-01-29 4:43 ` Cyril Bur 0 siblings, 0 replies; 8+ messages in thread From: Cyril Bur @ 2016-01-29 4:43 UTC (permalink / raw) To: Peng Fei BG Gou; +Cc: openbmc-patches, openbmc On Fri, 29 Jan 2016 04:23:21 +0000 "Peng Fei BG Gou" <shgoupf@cn.ibm.com> wrote: > Yes Cyril, your suggestion is fair, we should keep it consistent across all files in a project. There is always debating between tabs and whitespaces. Even though personally I prefer white spaces (that's why my editor automatically convert tab to whitespace for me), I agree that for this project, I should use tabs. Will make an update soon. So not being a Python person I was not all over PEP8 that Andrew mentioned (which states 4 space indenting), we should probably enforce that all new Python, even new entire functions/classes be PEP8 but single lines changes are just going to make the code illegible. Unfortunately we'll have to live with whats been written. I'll send a pull request to the docs I suppose. > > GOU, Peng Fei (苟鹏飞), Ph.D. > OpenPOWER Enablement. > +86-21-609-28631 > > > ----- Original message ----- > From: Cyril Bur <cyrilbur@gmail.com> > To: Peng Fei BG Gou/China/IBM@IBMCN > Cc: "openbmc" <openbmc@lists.ozlabs.org>, "OpenBMC Patches" <openbmc-patches@stwcx.xyz> > Subject: Re: 回复:回复:[PATCH phosphor-rest-server] The streaming support for obmc-rest. > Date: Fri, Jan 29, 2016 10:09 AM > > On Fri, 29 Jan 2016 00:49:39 +0000 > "Peng Fei BG Gou" <shgoupf@cn.ibm.com> wrote: > > > And the white space you saw in the email thread and coding is probably caused by the mixing use of tab and white space. The original code uses tab for indent, while I prefer to use 4 white spaces for indent. I will discuss with Brad regarding the indent convention for our code. Will fix it up if we strictly need tab in our project. > > In a world where we all have to read each others code, we should make an effort > to not make it hard for everyone. What you have done here is mix different > indenting styles which only makes it harder to read for everyone to read the > code. Imagine a world where I preferred 8 space indenting and I went through > and my additions to the file were 8 space indented, this would be impossible to > read. > > Typically when a file has been written in one way, it is left that way unless > that particular way is absolutely abhorrent (5 space indenting...). Tabs are a > perfectly valid way of indenting, I don't see the issue here. > > Futhurmore, I know this isn't really for python, if in doubt we should probably > be falling back to the openbmc/docs/contributing file and I quote: > > Components of the OpenBMC sources should have consistent style. > > For C code, we typically use the Linux coding style, which is > documented at: > > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/CodingStyle > > (unless you want to write a completely separate Python document, and change all > the python in this project, I suggest we follow this one where appropriate) > > Indent with tabs instead of spaces, set at 8 columns > > tl;dr > > Don't mix indenting styles, way too hard to read the code. > > > > > > > > > 在 2016年1月29日,上午8:38:15,"Peng Fei BG Gou" <shgoupf@cn.ibm.com> 写道: > > > > Python is indent based languang, so the function will fail if we have > > incorrect indenting. I have tested this in real bmc machine so I believe the > > indenting should be fine for now. Please let Brad review this change since he > > is familiar with Python. > > 在 2016年1月29日,上午8:24:58,"Cyril Bur" <cyrilbur@gmail.com> 写道: > > > > On Thu, 28 Jan 2016 02:00:37 -0600 > > OpenBMC Patches wrote: > > > From: shgoupf > > > > > Hi Peng, > > So I'm don't really know python all that well but I do believe this > > language is white space sensitive... I'll let a pythoner respond about the > > rest... > > > 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/ > > > --- > > > obmc-rest | 115 > > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file > > > changed, 104 insertions(+), 11 deletions(-) mode change 100644 => > > > 100755 obmc-rest > > > > > > diff --git a/obmc-rest b/obmc-rest > > > old mode 100644 > > > new mode 100755 > > > index c6d2949..481dafa > > > --- a/obmc-rest > > > +++ b/obmc-rest > > > @@ -3,7 +3,9 @@ > > > import os > > > import sys > > > import dbus > > > +import gobject > > > import dbus.exceptions > > > +import dbus.mainloop.glib > > > import json > > > import logging > > > from xml.etree import ElementTree > > > @@ -14,6 +16,10 @@ from OpenBMCMapper import Mapper, PathTree, > > > IntrospectionNodeParser, ListMatch import spwd > > > import grp > > > import crypt > > > +import threading > > > +import gevent > > > +from gevent.pywsgi import WSGIServer > > > +from gevent.queue import Queue > > > > > > DBUS_UNKNOWN_INTERFACE = 'org.freedesktop.UnknownInterface' > > > DBUS_UNKNOWN_METHOD = 'org.freedesktop.DBus.Error.UnknownMethod' > > > @@ -59,12 +65,13 @@ def makelist(data): > > > > > > class RouteHandler(object): > > > _require_auth = makelist(valid_user) > > > - def __init__(self, app, bus, verbs, rules): > > > + def __init__(self, app, bus, verbs, rules, skips = []): > > > self.app = app > > > self.bus = bus > > > self.mapper = Mapper(bus) > > > self._verbs = makelist(verbs) > > > self._rules = rules > > > + self._skips = skips > > > > > > def _setup(self, **kw): > > > request.route_data = {} > > > @@ -79,7 +86,7 @@ class RouteHandler(object): > > > return getattr(self, 'do_' + request.method.lower())(**kw) > > > > > > def install(self): > > > - self.app.route(self._rules, callback = self, > > > + self.app.route(self._rules, callback = self, skip = self._skips, > > > method = ['GET', 'PUT', 'PATCH', 'POST', 'DELETE']) > > > > > > @staticmethod > > > @@ -108,6 +115,58 @@ class RouteHandler(object): > > > return None > > > raise > > > > > > +class SignalHandler(RouteHandler): > > > + verbs = ['GET'] > > > + rules = '/stream/' > > > + > > > + def __init__(self, app, bus): > > > + super(SignalHandler, self).__init__( > > > + app, bus, self.verbs, self.rules) > > > + > > > + def find(self, path, signal): > > > + busses = self.try_mapper_call(self.mapper.get_object, > > > + path = path) > > > + for items in busses.iteritems(): > > > + s = self.find_signal_on_bus(path, signal, *items) > > > + if s: > > > + return s > > > + > > > + abort(404, _4034_msg %('signal', 'found', signal)) > > > + > > > + def setup(self, path, signal): > > > + request.route_data['map'] = self.find(path, signal) > > > + > > > + def do_get(self, path, signal): > > > + body = Queue() > > > + dsignal = DbusSignal(bus, request.route_data['map'][0], > > > + request.route_data['map'][1], > > > path) > > > + dsignal.onData(body.put) > > > + dsignal.onFinish(lambda: body.put(StopIteration)) > > > + dsignal.signalSnooping() > > > + return body > > > + > > > + @staticmethod > > > + def find_signal(signal, signals): > > > + if signals is None: > > > + return None > > > + > > > + signal = find_case_insensitive(signal, signals.keys()) > > > + if signal is not None: > > > + return signal > > > + > > > + def find_signal_on_bus(self, path, signal, bus, interfaces): > > > + obj = self.bus.get_object(bus, path, introspect = False) > > > + iface = dbus.Interface(obj, dbus.INTROSPECTABLE_IFACE) > > > + data = iface.Introspect() > > > + parser = IntrospectionNodeParser( > > > + ElementTree.fromstring(data), > > > + intf_match = ListMatch(interfaces)) > > > + for x,y in parser.get_interfaces().iteritems(): > > > + s = self.find_signal(signal, > > > + y.get('signal')) > > > + if s: > > > + return (x,s) > > > + > > > class DirectoryHandler(RouteHandler): > > > verbs = 'GET' > > > rules = '/' > > > @@ -715,7 +774,8 @@ class RestApp(Bottle): > > > self.install(JSONPlugin(**json_kw)) > > > self.install(JsonApiErrorsPlugin(**json_kw)) > > > self.install(AuthorizationPlugin()) > > > - self.install(JsonApiResponsePlugin()) > > > + self.json_response_plugin = JsonApiResponsePlugin() > > Indenting? > > > + self.install(self.json_response_plugin) > > > self.install(JsonApiRequestPlugin()) > > > self.install(JsonApiRequestTypePlugin()) > > > > > > @@ -726,6 +786,7 @@ class RestApp(Bottle): > > > > > > def create_handlers(self): > > > # create route handlers > > > + self.signal_handler = SignalHandler(self, self.bus) > > > self.session_handler = SessionHandler(self, self.bus) > > > self.directory_handler = DirectoryHandler(self, self.bus) > > > self.list_names_handler = ListNamesHandler(self, self.bus) > > > @@ -736,6 +797,11 @@ class RestApp(Bottle): > > > self.instance_handler = InstanceHandler(self, self.bus) > > > > > > def install_handlers(self): > > > + # Skip json response for signal handler because it > > > requires to > > > + # return a gevent iterable which cannot be handled by > > > json > > > + # response plugin > > > + self.signal_handler._skips = > > > [self.json_response_plugin] > > Indenting? > > > + self.signal_handler.install() > > > self.session_handler.install() > > > self.directory_handler.install() > > > self.list_names_handler.install() > > > @@ -766,21 +832,48 @@ class RestApp(Bottle): > > > parts = filter(bool, path.split('/')) > > > request.environ['PATH_INFO'] = '/' + '/'.join(parts) + trailing > > > > > > +class DbusSignal(): > > > + def __init__(self, bus, dbus_interface, signal_name, path): > > > + # Register the dbus recieve handler > > > + bus.add_signal_receiver(self.signalReciever, > > > + dbus_interface = dbus_interface, > > > + signal_name = signal_name, > > > + path = path) > > > + > > > + self.snooping = True > > > + > > > + def signalReciever(self, msg): > > > + self.send("Recieved message: %s" % msg) > > > + self.snooping = False > > > + > > > + def onData(self, send): > > > + self.send = send > > > + > > > + def onFinish(self, f): > > > + self.finish = f > > > + > > > + def signalSnooping(self): > > > + while self.snooping: > > > + mainloop = gobject.MainLoop() > > > + gevent.sleep(1) > > > + gobject.timeout_add(1, mainloop.quit) > > > + mainloop.run() > > > + > > > + self.finish() > > > + > > > if __name__ == '__main__': > > > log = logging.getLogger('Rocket.Errors') > > > log.setLevel(logging.INFO) > > > log.addHandler(logging.StreamHandler(sys.stdout)) > > > > > > + dbus.mainloop.glib.DBusGMainLoop(set_as_default=True) > > Indenting? > > > bus = dbus.SystemBus() > > > app = RestApp(bus) > > > + > > ? > > > default_cert = os.path.join(sys.prefix, 'share', > > > os.path.basename(__file__), 'cert.pem') > > > > > > - server = Rocket(('0.0.0.0', > > > - 443, > > > - default_cert, > > > - default_cert), > > > - 'wsgi', {'wsgi_app': app}, > > > - min_threads = 1, > > > - max_threads = 1) > > > - server.start() > > > + server = WSGIServer(("0.0.0.0", 443), app, keyfile = > > > default_cert, > > > + certfile = default_cert) > > > + > > > + server.serve_forever() > > Indenting? > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-01-29 6:57 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <OF66059493.03804909-ON00257F49.000380B3-1454027895800@LocalDomain>
2016-01-29 0:49 ` 回复:回复:[PATCH phosphor-rest-server] The streaming support for obmc-rest Peng Fei BG Gou
[not found] ` <201601290049.u0T0nquh003020@d23av02.au.ibm.com>
2016-01-29 2:08 ` Cyril Bur
2016-01-29 3:27 ` Andrew Donnellan
2016-01-29 4:23 ` Peng Fei BG Gou
2016-01-29 6:17 ` Vishwanatha Subbanna
2016-01-29 6:54 ` Manjunath A Kumatagi
2016-01-29 6:57 ` Manjunath A Kumatagi
[not found] ` <201601290423.u0T4NYkd013405@d23av02.au.ibm.com>
2016-01-29 4:43 ` Cyril Bur
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.