12:17 a.m.
Please review the patch which implements the remaining of the
ProfileClient API for create/modify/delete profiles.
Includes basic test cases in the main method of profile.py module.
-- Abhishek
2:50 a.m.
On Wed, Jun 18, 2014 at 01:17:23AM -0400, Abhishek Koneru wrote:
Please review the patch which implements the remaining of the
ProfileClient API for create/modify/delete profiles.
Includes basic test cases in the main method of profile.py module.
-- Abhishek
Applied the patch and ran the main method to test. Does what it
says on the tin, but haven't rigorously tested it / used it in anger
yet.
Patch looks fine, just one nitpick (see below).
Cheers,
Fraser
diff --git a/base/common/python/pki/__init__.py
b/base/common/python/pki/__init__.py
index 891d6ea6364b037f132ff3754b73b372c638b0f7..e9b726cf763785b4a700ef314ff27774b13aba40
100644
--- a/base/common/python/pki/__init__.py
+++ b/base/common/python/pki/__init__.py
@@ -168,7 +168,7 @@ class PKIException(Exception, ResourceMessage):
ret = cls(json_value['Message'], json_value['Code'],
json_value['ClassName'])
for attr in json_value['Attributes']['Attribute']:
- print str(attr)
+ print(str(attr))
ret.add_attribute(attr["name"], attr["value"])
return ret
@@ -299,7 +299,7 @@ class PropertyFile(object):
def show(self):
""" Show contents of property file."""
for line in self.lines:
- print line
+ print(line)
Nit-pick: Why the parens? Even if this is the correct style, it is
unrelated to the other changes so might be better to include in a
separate patch?
1:56 p.m.
In general, patch looks good.
1. Are the parens a PEP8/ pycharm/ pylint thing? If not, I'm not sure I
see the point of adding them.
2. profile.py needs the standard RH copyright notice at the top.
3. I'd like to see a little more testing in the main(). In particular,
I'd like to see the use of Profile Outputs and ProfilePolicySets.
Ade
On Wed, 2014-06-18 at 17:50 +1000, Fraser Tweedale wrote:
On Wed, Jun 18, 2014 at 01:17:23AM -0400, Abhishek Koneru wrote:
> Please review the patch which implements the remaining of the
> ProfileClient API for create/modify/delete profiles.
>
> Includes basic test cases in the main method of profile.py module.
>
> -- Abhishek
Applied the patch and ran the main method to test. Does what it
says on the tin, but haven't rigorously tested it / used it in anger
yet.
Patch looks fine, just one nitpick (see below).
Cheers,
Fraser
> diff --git a/base/common/python/pki/__init__.py
b/base/common/python/pki/__init__.py
> index
891d6ea6364b037f132ff3754b73b372c638b0f7..e9b726cf763785b4a700ef314ff27774b13aba40 100644
> --- a/base/common/python/pki/__init__.py
> +++ b/base/common/python/pki/__init__.py
> @@ -168,7 +168,7 @@ class PKIException(Exception, ResourceMessage):
> ret = cls(json_value['Message'], json_value['Code'],
> json_value['ClassName'])
> for attr in json_value['Attributes']['Attribute']:
> - print str(attr)
> + print(str(attr))
> ret.add_attribute(attr["name"], attr["value"])
> return ret
>
> @@ -299,7 +299,7 @@ class PropertyFile(object):
> def show(self):
> """ Show contents of property file."""
> for line in self.lines:
> - print line
> + print(line)
>
Nit-pick: Why the parens? Even if this is the correct style, it is
unrelated to the other changes so might be better to include in a
separate patch?
_______________________________________________
Pki-devel mailing list
Pki-devel(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel
3834
days inactive
3839
days old
2 comments
3 participants
participants (3)
-
Abhishek Koneru -
Ade Lee -
Fraser Tweedale