Re: 回复：回复：[PATCH phosphor-rest-server] The streaming support for obmc-rest.
Manjunath A Kumatagi
mkumatag at in.ibm.com
Fri Jan 29 17:57:35 AEDT 2016
link for PEP8 - https://www.python.org/dev/peps/pep-0008/
From: Manjunath A Kumatagi/India/IBM at IBMIN
To: Vishwanatha Subbanna/India/IBM at IBMIN
Cc: openbmc at lists.ozlabs.org, openbmc-patches at stwcx.xyz,
cyrilbur at gmail.com, Peng Fei BG Gou <shgoupf at 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 at 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 at IBMIN
To: "Peng Fei BG Gou" <shgoupf at cn.ibm.com>
Cc: openbmc at lists.ozlabs.org, openbmc-patches at stwcx.xyz, cyrilbur at 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 at 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 at 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 at cn.ibm.com>
To: cyrilbur at gmail.com
Cc: openbmc at lists.ozlabs.org, openbmc-patches at 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 at 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 at gmail.com>
To: Peng Fei BG Gou/China/IBM at IBMCN
Cc: "openbmc" <openbmc at lists.ozlabs.org>, "OpenBMC Patches"
<openbmc-patches at 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 at 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 at 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 at 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 at lists.ozlabs.org
https://lists.ozlabs.org/listinfo/openbmc
_______________________________________________
openbmc mailing list
openbmc at lists.ozlabs.org
https://lists.ozlabs.org/listinfo/openbmc
_______________________________________________
openbmc mailing list
openbmc at lists.ozlabs.org
https://lists.ozlabs.org/listinfo/openbmc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20160129/4ba67ced/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: graycol.gif
Type: image/gif
Size: 105 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20160129/4ba67ced/attachment-0001.gif>
More information about the openbmc
mailing list