Hi, Thanks for the tip. I have gone through the pylint results for the code I'm adding below and fixed a number of style issues and two bugs discovered by the linter. I will include these next time I re-send the updated changeset. There are remaining issues reported by pylint that I haven't fixed because it's not obvious that they create more readable code. An example where pylint is inconvenient is that it doesn't allow "dn" as a variable name. :-)
/ Johan On 09/16/2015 10:23 AM, Hans Nordebäck wrote: > Hi, > > A general comment, there is a ticket > https://sourceforge.net/p/opensaf/tickets/751/ pyosaf: fix bad pylint > rating. Have you run pylint after these changes? /Thanks HansN > > -----Original Message----- > From: Johan Mårtensson O > Sent: den 15 september 2015 14:34 > To: Hans Nordebäck; [email protected]; Hung Nguyen D; > [email protected] > Cc: [email protected] > Subject: [PATCH 00 of 17] Review Request for pyosaf: [Round 6] Add Python imm > oi utils module and sample applications [#1406] > > Summary: pyosaf: [Round 6] Add Python imm oi utils module and sample > applications [#1406] Review request for Trac Ticket(s): #1406 Peer > Reviewer(s): [email protected], [email protected], > [email protected], [email protected] Pull request to: > [email protected] Affected branch(es): 4.7.x Development branch: > opensaf-staging > > -------------------------------- > Impacted area Impact y/n > -------------------------------- > Docs n > Build system n > RPM/packaging n > Configuration files n > Startup scripts n > SAF services n > OpenSAF services n > Core libraries n > Samples n > Tests n > Other y > > > Comments (indicate scope for each "y" above): > --------------------------------------------- > Updated after review comments from Hung Nguyen. The main changes are: > > - Change admin op upcall to include parameter name and type > - Fix DN handling and class lookup in the cardinality validation > - Minor fixes to the sample OIs > > > changeset 763aff55586eb30e5397e992da65b1c4581f475a > Author: Johan Mårtensson <[email protected]> > Date: Thu, 20 Aug 2015 13:13:41 +0200 > > pyosaf: Add Python imm oi utils module and sample applications [#1406] > > Add high-level imm oi utils module on top of the direct imm oi Python > bindings. Also add sample applications demonstrating usage both as > subclassing and with direct callbacks. > > changeset 4175ba199c25cdba2d67815e555d074c7f782188 > Author: Johan Mårtensson <[email protected]> > Date: Thu, 27 Aug 2015 15:23:53 +0200 > > pyosaf: Fix handling of attribute updates and associated sample > applications > [#1406] > > Fix the handling of updates of runtime attributes on request from IMM. > Add > sample application to demonstrate this, both for direct callbacks and by > using a subclass. > > Verify by executing the sample applications: > > ./users & > > immlist usersId=1 > > or > > ./users-inheritance-impl & > > immlist usersId=2 > > changeset 1a95fca3d08330a9fbf2accd3365fae6ccf394b4 > Author: Johan Mårtensson <[email protected]> > Date: Fri, 28 Aug 2015 11:31:48 +0200 > > pyosaf: Make the users attribute in the UsersSampleClass multivalued > [#1406] > > Make the users attribute in the UsersSampleClass multivalued. > > changeset aa808e57b2504b12d13905561ff78b9ae25e9c0e > Author: Johan Mårtensson <[email protected]> > Date: Fri, 28 Aug 2015 11:33:03 +0200 > > pyosaf: Correct the users inheritance sample OI to use the right IMM > object > [#1406] > > Correct the inheritance implementation of the users sample OI to use the > right IMM object. > > changeset da46babee9d83691553d4b6b872664c822bdbd91 > Author: Johan Mårtensson <[email protected]> > Date: Fri, 28 Aug 2015 11:33:33 +0200 > > pyosaf: Add the users sample OI to the README [#1406] > > Add the users sample OI to the README > > changeset 6da0d9897c710f5035056571c7ea1e55cf5ac3f2 > Author: Johan Mårtensson <[email protected]> > Date: Fri, 28 Aug 2015 15:20:32 +0200 > > pyosaf: Define DN before using it to filter [#1406] > > When building the full content of the CCB on completed, a dn variable of > create was used without defining it. This patch fixes it. > > changeset 4d076b5d92002076f84aa9376205c6f0205bf756 > Author: Johan Mårtensson <[email protected]> > Date: Thu, 03 Sep 2015 16:39:41 +0200 > > pyosaf: Make 'deleted' contain objects and fix containment code [#1406] > > Fix the 'deleted' list passed in validate and apply callbacks to contain > proper instances of ImmObject instead of just DNs. Also fix the > containment > validation. > > changeset 6f764adb745cf67c3ed87e3e75b4aed99d3e7859 > Author: Johan Mårtensson <[email protected]> > Date: Thu, 03 Sep 2015 17:07:07 +0200 > > pyosaf: Move sample OIs to the correct directory and merge READMEs > [#1406] > > Move the sample OIs to the same directory as the existing samples. Also > merge the separate OI README with the existing README for samples. > > changeset 74ed5e02d006cee62dfd7a9d44dce34305f91a05 > Author: Johan Mårtensson <[email protected]> > Date: Thu, 03 Sep 2015 17:13:29 +0200 > > pyosaf: Add immoi utils to the makefiles and rpm spec file [#1406] > > Add the immoi utils to the makefiles and the rpm spec file. > > changeset 40bb6a3c21e480619d0fb3e491bd9d90aef883a1 > Author: Johan Mårtensson <[email protected]> > Date: Mon, 14 Sep 2015 11:28:54 +0200 > > pyosaf: Use SA_IMM_SEARCH_GET_CONFIG_ATTR to exclude runtime attributes > [#1406] > > Use the SA_IMM_SEARCH_GET_CONFIG_ATTR flag in > immoi.get_object_no_runtime() > to only query config attributes and update immom.get() to handle it. > > Verify by querying an object with both config and runtime attributes: > > from pyosaf.utils import immoi print immoi.get_object_no_runtime('<DN>') > > changeset deac33a77ce537fbbe47107ebdc41350019eb987 > Author: Johan Mårtensson <[email protected]> > Date: Mon, 14 Sep 2015 12:59:43 +0200 > > pyosaf: Fix the _validate() method in Applier to include the CCB id in > its > arguments [#1406] > > Correct the _validate() method in Applier to take the CCB id as its > first > argument. > > changeset cf63b81c46b44bfcd5230ceb86dc63d15f3f7e56 > Author: Johan Mårtensson <[email protected]> > Date: Mon, 14 Sep 2015 15:06:17 +0200 > > pyosaf: Correct interface-handler sample OI and add to README [#1406] > > Correct the interface-handler according to review comments, move it to > the > right directory, and add a description of it to the README. > > changeset ca9f16b565d07f58ac17e2f813966f2b51c172a7 > Author: Johan Mårtensson <[email protected]> > Date: Mon, 14 Sep 2015 15:13:46 +0200 > > pyosaf: Correct cardinality validation and validate 'deleted' argument > [#1406] > > Correct the cardinality validation and the 'deleted' argument to the > validate callback according to review comments. The deleted MOs were > passed > as ImmObjects but this doesn't work for the applier case so now they are > passed as DNs instead. > > changeset a78b3ce21e8464580a9c83dc186e1df637ac5453 > Author: Johan Mårtensson <[email protected]> > Date: Mon, 14 Sep 2015 15:16:53 +0200 > > pyosaf: Finish merge of samples and immoi/samples [#1406] > > Finish the merge of the previously separate immoi/samples directory > with the > other samples. > > changeset 9d95ee15f04b4205b0c5dde631acc139ab39f089 > Author: Johan Mårtensson <[email protected]> > Date: Mon, 14 Sep 2015 16:26:02 +0200 > > pyosaf: Correct sample applications and add help text [#1406] > > Correct mistakes in the sample applications and add proper help text to > all. > > changeset ce9e0080dbbf7036754ca487e359a62a3430e447 > Author: Johan Mårtensson <[email protected]> > Date: Tue, 15 Sep 2015 11:58:33 +0200 > > pyosaf: Include admin op parameter name and type in upcall and minor > corrections after review [#1406] > > Update according to review comments: > > - Add AdminOperationParameter class and use it to pass name, type and > value > for each parameter in the upcall > - Update ping-pong sample OI to use the AdminOperationParameter > - Correct creation of root-level DNs and lookup of parent's class in > the > completed upcall where the parent is not yet created. > > changeset d627cb18ac0e1ca118d7e443af95b35c42ae7ed7 > Author: Johan Mårtensson <[email protected]> > Date: Tue, 15 Sep 2015 14:12:40 +0200 > > pyosaf: Minor fixes to the caps and interface sample OIs [#1406] > > Fix minor issues in the caps and interface sample OIs > > > Added Files: > ------------ > python/pyosaf/utils/immoi/implementer.py > python/pyosaf/utils/immoi/__init__.py > python/pyosaf/utils/immoi/Makefile.am > python/samples/caps > python/samples/caps-inheritance-impl > python/samples/imm-listener > python/samples/imm-listener-inheritance-impl > python/samples/immoi/samples/caps > python/samples/immoi/samples/caps-inheritance-impl > python/samples/immoi/samples/classes.xml > python/samples/immoi/samples/imm-listener > python/samples/immoi/samples/imm-listener-inheritance-impl > python/samples/immoi/samples/interface-handler > python/samples/immoi/samples/interface-handler-inheritance-version > python/samples/immoi/samples/ping-pong > python/samples/immoi/samples/ping-pong-inheritance-impl > python/samples/immoi/samples/README > python/samples/immoi/samples/time-reporter > python/samples/immoi/samples/time-reporter-inheritance-impl > python/samples/immoi/samples/tones > python/samples/immoi/samples/tones-inheritance-impl > python/samples/immoi/samples/users > python/samples/immoi/samples/users-inheritance-impl > python/samples/ping-pong > python/samples/ping-pong-inheritance-impl > python/samples/time-reporter > python/samples/time-reporter-inheritance-impl > python/samples/tones > python/samples/tones-inheritance-impl > python/samples/users > python/samples/users-inheritance-impl > > > Removed Files: > -------------- > python/samples/immoi/samples/caps > python/samples/immoi/samples/caps-inheritance-impl > python/samples/immoi/samples/imm-listener > python/samples/immoi/samples/imm-listener-inheritance-impl > python/samples/immoi/samples/interface-handler > python/samples/immoi/samples/interface-handler-inheritance-version > python/samples/immoi/samples/ping-pong > python/samples/immoi/samples/ping-pong-inheritance-impl > python/samples/immoi/samples/README > python/samples/immoi/samples/time-reporter > python/samples/immoi/samples/time-reporter-inheritance-impl > python/samples/immoi/samples/tones > python/samples/immoi/samples/tones-inheritance-impl > python/samples/immoi/samples/users > python/samples/immoi/samples/users-inheritance-impl > > > Complete diffstat: > ------------------ > Makefile.common | 1 + > configure.ac | 1 + > opensaf.spec.in | 3 + > python/pyosaf/utils/Makefile.am | 3 +- > python/pyosaf/utils/immoi/Makefile.am | 23 + > python/pyosaf/utils/immoi/__init__.py | 394 > +++++++++++++++++++++ > python/pyosaf/utils/immoi/implementer.py | 934 > ++++++++++++++++++++++++++++++++++++++++++++++++++ > python/pyosaf/utils/immom/__init__.py | 12 +- > python/samples/README | 56 ++ > python/samples/caps | 51 ++ > python/samples/caps-inheritance-impl | 58 +++ > python/samples/imm-listener | 111 +++++ > python/samples/imm-listener-inheritance-impl | 158 ++++++++ > python/samples/interface-handler | 130 ++++++ > python/samples/interface-handler-inheritance-impl | 133 +++++++ > python/samples/ping-pong | 78 ++++ > python/samples/ping-pong-inheritance-impl | 72 +++ > python/samples/sample-classes.xml | 203 ++++++++++ > python/samples/time-reporter | 75 ++++ > python/samples/time-reporter-inheritance-impl | 90 ++++ > python/samples/tones | 27 + > python/samples/tones-inheritance-impl | 32 + > python/samples/users | 39 ++ > python/samples/users-inheritance-impl | 43 ++ > 24 files changed, 2725 insertions(+), 2 deletions(-) > > > Testing Commands: > ----------------- > <<LIST THE COMMAND LINE TOOLS/STEPS TO TEST YOUR CHANGES>> > > > Testing, Expected Results: > -------------------------- > <<PASTE COMMAND OUTPUTS / TEST RESULTS>> > > > Conditions of Submission: > ------------------------- > <<HOW MANY DAYS BEFORE PUSHING, CONSENSUS ETC>> > > > Arch Built Started Linux distro > ------------------------------------------- > mips n n > mips64 n n > x86 n n > x86_64 n n > powerpc n n > powerpc64 n n > > > Reviewer Checklist: > ------------------- > [Submitters: make sure that your review doesn't trigger any checkmarks!] > > > Your checkin has not passed review because (see checked entries): > > ___ Your RR template is generally incomplete; it has too many blank entries > that need proper data filled in. > > ___ You have failed to nominate the proper persons for review and push. > > ___ Your patches do not have proper short+long header > > ___ You have grammar/spelling in your header that is unacceptable. > > ___ You have exceeded a sensible line length in your headers/comments/text. > > ___ You have failed to put in a proper Trac Ticket # into your commits. > > ___ You have incorrectly put/left internal data in your comments/files > (i.e. internal bug tracking tool IDs, product names etc) > > ___ You have not given any evidence of testing beyond basic build tests. > Demonstrate some level of runtime or other sanity testing. > > ___ You have ^M present in some of your files. These have to be removed. > > ___ You have needlessly changed whitespace or added whitespace crimes > like trailing spaces, or spaces before tabs. > > ___ You have mixed real technical changes with whitespace and other > cosmetic code cleanup changes. These have to be separate commits. > > ___ You need to refactor your submission into logical chunks; there is > too much content into a single commit. > > ___ You have extraneous garbage in your review (merge commits etc) > > ___ You have giant attachments which should never have been sent; > Instead you should place your content in a public tree to be pulled. > > ___ You have too many commits attached to an e-mail; resend as threaded > commits, or place in a public tree for a pull. > > ___ You have resent this content multiple times without a clear indication > of what has changed between each re-send. > > ___ You have failed to adequately and individually address all of the > comments and change requests that were proposed in the initial review. > > ___ You have a misconfigured ~/.hgrc file (i.e. username, email etc) > > ___ Your computer have a badly configured date and time; confusing the > the threaded patch review. > > ___ Your changes affect IPC mechanism, and you don't present any results > for in-service upgradability test. > > ___ Your changes affect user manual and documentation, your patch series > do not contain the patch that updates the Doxygen manual. > ------------------------------------------------------------------------------ Monitor Your Dynamic Infrastructure at Any Scale With Datadog! Get real-time metrics from all of your servers, apps and tools in one place. SourceForge users - Click here to start your Free Trial of Datadog now! http://pubads.g.doubleclick.net/gampad/clk?id=241902991&iu=/4140 _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
