Re: [Freeipa-devel] [PATCH] pylint fixes
On 01.07.2016 15:51, Florence Blanc-Renaud wrote:
On 06/21/2016 01:51 PM, Martin Basti wrote:
On 21.06.2016 08:38, Florence Blanc-Renaud wrote:
On 06/20/2016 07:08 PM, Martin Basti wrote:
On 20.06.2016 19:06, Martin Basti wrote:
On 20.06.2016 12:00, Florence Blanc-Renaud wrote:
On 06/09/2016 05:10 PM, Petr Spacek wrote:
Hello,
I've received a bunch of pylint fixes produced by upstream
contributor who is
not subscribed to the list so I'm resending them here.
All credit goes to Bárta Jan <55042ba...@sstebrno.eu>.
Flo, if you have time for it I think that it could be a good
exercise which
will lead you to various dark corners in IPA :-)
Petr^2 Spacek
Forwarded Message
Date: Fri, 3 Jun 2016 14:57:16 +0200
From: Bárta Jan <55042ba...@sstebrno.eu>
To: pspa...@redhat.com
___- In the patch
0002-pylint-fix-simplifiable-if-statement-warnings.patch:_
diff --git a/ipatests/test_integration/tasks.py
b/ipatests/test_integration/tasks.py
index aebd907..ca2e10f 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -149,11 +149,7 @@ def host_service_active(host, service):
res = host.run_command(['systemctl', 'is-active', '--quiet',
service],
raiseonerr=False)
-if res.returncode == 0:
-return True
-else:
-return False
-
+return res.returncode
should be instead: return res.returncode *== 0* (otherwise the
return
type is an int and not a boolean).
In the same file:
@@ -295,11 +291,7 @@ def
master_authoritative_for_client_domain(master, client):
zone = ".".join(client.hostname.split('.')[1:])
result = master.run_command(["ipa", "dnszone-show", zone],
raiseonerr=False)
-if result.returncode == 0:
-return True
-else:
-return False
-
+result.returncode == 0
should be instead: *return* result.returncode == 0 (otherwise there
is no return statement)
diff --git a/ipaserver/plugins/dogtag.py
b/ipaserver/plugins/dogtag.py
index 197814c..36b6ba5 100644
--- a/ipaserver/plugins/dogtag.py
+++ b/ipaserver/plugins/dogtag.py
@@ -1689,12 +1689,7 @@ class ra(rabase.rabase):
# Return command result
cmd_result = {}
-if parse_result.get('revoked') == 'yes':
-cmd_result['revoked'] = True
-else:
-cmd_result['revoked'] = False
-
-return cmd_result
+cmd_result['revoked'] = parse_result.get('revoked')
Should be instead: cmd_result['revoked'] =
parse_result.get('revoked') *== 'yes'* (otherwise the type is a
string and not a boolean)
_- in the patch 00__04-pylint-fix-unneeded-not.patch_
@@ -632,7 +632,7 @@ class host_add(LDAPCreate):
options['ip_address'],
check_forward=True,
check_reverse=check_reverse)
-if not options.get('force', False) and not 'ip_address' in
options:
+if options.get('force', False) and 'ip_address' not in
options:
Should be instead: if *not* options.get('force', False) and
'ip_address' not in options:
because of operators precedence
I will review patches 0005 to 0010 later today.
Flo.
How about patches 1, and 3? Because patches are independent, we can
separately ACK them and push them.
Martin^2
Sorry, I just noticed that there is no patch 1 :)
Patch 0003 is OK, ACK for this one.
Flo.
Patch 0003: Pushed to master: 94909d21dbf033cbe34089782c430ec25b9ad0bc
Hi,
please find my comments on the remaining patches.
- Patch 0005 must be rebased because of changes in
ipatests/test_integration/tasks.py
the patch can also modify pylintrc (remove pointless-statement)
- Patch 0006:
no need to rename the items in "for e in ...", renaming the Exception
as exc should be enough
- Patch 0007:
pylintrc should remove old-style-class instead of
bad-classmethod-argument
- Patch 0008:
this one should remove bad-classmethod-argument in pylintrc
- Patch 0009:
ok
- Patch 0010:
In the __bind method(self, obj_type), cls variable is already used
thus replacing self with cls can be done only if cls is also renamed
into something else.
Flo.
Please follow new changes in this PR
https://github.com/freeipa/freeipa/pull/97
It will be easier to review.
Martin^2
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] pylint fixes
On 06/21/2016 01:51 PM, Martin Basti wrote:
On 21.06.2016 08:38, Florence Blanc-Renaud wrote:
On 06/20/2016 07:08 PM, Martin Basti wrote:
On 20.06.2016 19:06, Martin Basti wrote:
On 20.06.2016 12:00, Florence Blanc-Renaud wrote:
On 06/09/2016 05:10 PM, Petr Spacek wrote:
Hello,
I've received a bunch of pylint fixes produced by upstream
contributor who is
not subscribed to the list so I'm resending them here.
All credit goes to Bárta Jan <55042ba...@sstebrno.eu>.
Flo, if you have time for it I think that it could be a good
exercise which
will lead you to various dark corners in IPA :-)
Petr^2 Spacek
Forwarded Message
Date: Fri, 3 Jun 2016 14:57:16 +0200
From: Bárta Jan <55042ba...@sstebrno.eu>
To: pspa...@redhat.com
___- In the patch
0002-pylint-fix-simplifiable-if-statement-warnings.patch:_
diff --git a/ipatests/test_integration/tasks.py
b/ipatests/test_integration/tasks.py
index aebd907..ca2e10f 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -149,11 +149,7 @@ def host_service_active(host, service):
res = host.run_command(['systemctl', 'is-active', '--quiet',
service],
raiseonerr=False)
-if res.returncode == 0:
-return True
-else:
-return False
-
+return res.returncode
should be instead: return res.returncode *== 0* (otherwise the return
type is an int and not a boolean).
In the same file:
@@ -295,11 +291,7 @@ def
master_authoritative_for_client_domain(master, client):
zone = ".".join(client.hostname.split('.')[1:])
result = master.run_command(["ipa", "dnszone-show", zone],
raiseonerr=False)
-if result.returncode == 0:
-return True
-else:
-return False
-
+result.returncode == 0
should be instead: *return* result.returncode == 0 (otherwise there
is no return statement)
diff --git a/ipaserver/plugins/dogtag.py b/ipaserver/plugins/dogtag.py
index 197814c..36b6ba5 100644
--- a/ipaserver/plugins/dogtag.py
+++ b/ipaserver/plugins/dogtag.py
@@ -1689,12 +1689,7 @@ class ra(rabase.rabase):
# Return command result
cmd_result = {}
-if parse_result.get('revoked') == 'yes':
-cmd_result['revoked'] = True
-else:
-cmd_result['revoked'] = False
-
-return cmd_result
+cmd_result['revoked'] = parse_result.get('revoked')
Should be instead: cmd_result['revoked'] =
parse_result.get('revoked') *== 'yes'* (otherwise the type is a
string and not a boolean)
_- in the patch 00__04-pylint-fix-unneeded-not.patch_
@@ -632,7 +632,7 @@ class host_add(LDAPCreate):
options['ip_address'],
check_forward=True,
check_reverse=check_reverse)
-if not options.get('force', False) and not 'ip_address' in
options:
+if options.get('force', False) and 'ip_address' not in
options:
Should be instead: if *not* options.get('force', False) and
'ip_address' not in options:
because of operators precedence
I will review patches 0005 to 0010 later today.
Flo.
How about patches 1, and 3? Because patches are independent, we can
separately ACK them and push them.
Martin^2
Sorry, I just noticed that there is no patch 1 :)
Patch 0003 is OK, ACK for this one.
Flo.
Patch 0003: Pushed to master: 94909d21dbf033cbe34089782c430ec25b9ad0bc
Hi,
please find my comments on the remaining patches.
- Patch 0005 must be rebased because of changes in
ipatests/test_integration/tasks.py
the patch can also modify pylintrc (remove pointless-statement)
- Patch 0006:
no need to rename the items in "for e in ...", renaming the Exception as
exc should be enough
- Patch 0007:
pylintrc should remove old-style-class instead of bad-classmethod-argument
- Patch 0008:
this one should remove bad-classmethod-argument in pylintrc
- Patch 0009:
ok
- Patch 0010:
In the __bind method(self, obj_type), cls variable is already used thus
replacing self with cls can be done only if cls is also renamed into
something else.
Flo.
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] pylint fixes
On 21.06.2016 08:38, Florence Blanc-Renaud wrote:
On 06/20/2016 07:08 PM, Martin Basti wrote:
On 20.06.2016 19:06, Martin Basti wrote:
On 20.06.2016 12:00, Florence Blanc-Renaud wrote:
On 06/09/2016 05:10 PM, Petr Spacek wrote:
Hello,
I've received a bunch of pylint fixes produced by upstream
contributor who is
not subscribed to the list so I'm resending them here.
All credit goes to Bárta Jan <55042ba...@sstebrno.eu>.
Flo, if you have time for it I think that it could be a good
exercise which
will lead you to various dark corners in IPA :-)
Petr^2 Spacek
Forwarded Message
Date: Fri, 3 Jun 2016 14:57:16 +0200
From: Bárta Jan <55042ba...@sstebrno.eu>
To: pspa...@redhat.com
___- In the patch
0002-pylint-fix-simplifiable-if-statement-warnings.patch:_
diff --git a/ipatests/test_integration/tasks.py
b/ipatests/test_integration/tasks.py
index aebd907..ca2e10f 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -149,11 +149,7 @@ def host_service_active(host, service):
res = host.run_command(['systemctl', 'is-active', '--quiet',
service],
raiseonerr=False)
-if res.returncode == 0:
-return True
-else:
-return False
-
+return res.returncode
should be instead: return res.returncode *== 0* (otherwise the return
type is an int and not a boolean).
In the same file:
@@ -295,11 +291,7 @@ def
master_authoritative_for_client_domain(master, client):
zone = ".".join(client.hostname.split('.')[1:])
result = master.run_command(["ipa", "dnszone-show", zone],
raiseonerr=False)
-if result.returncode == 0:
-return True
-else:
-return False
-
+result.returncode == 0
should be instead: *return* result.returncode == 0 (otherwise there
is no return statement)
diff --git a/ipaserver/plugins/dogtag.py b/ipaserver/plugins/dogtag.py
index 197814c..36b6ba5 100644
--- a/ipaserver/plugins/dogtag.py
+++ b/ipaserver/plugins/dogtag.py
@@ -1689,12 +1689,7 @@ class ra(rabase.rabase):
# Return command result
cmd_result = {}
-if parse_result.get('revoked') == 'yes':
-cmd_result['revoked'] = True
-else:
-cmd_result['revoked'] = False
-
-return cmd_result
+cmd_result['revoked'] = parse_result.get('revoked')
Should be instead: cmd_result['revoked'] =
parse_result.get('revoked') *== 'yes'* (otherwise the type is a
string and not a boolean)
_- in the patch 00__04-pylint-fix-unneeded-not.patch_
@@ -632,7 +632,7 @@ class host_add(LDAPCreate):
options['ip_address'],
check_forward=True,
check_reverse=check_reverse)
-if not options.get('force', False) and not 'ip_address' in
options:
+if options.get('force', False) and 'ip_address' not in
options:
Should be instead: if *not* options.get('force', False) and
'ip_address' not in options:
because of operators precedence
I will review patches 0005 to 0010 later today.
Flo.
How about patches 1, and 3? Because patches are independent, we can
separately ACK them and push them.
Martin^2
Sorry, I just noticed that there is no patch 1 :)
Patch 0003 is OK, ACK for this one.
Flo.
Patch 0003: Pushed to master: 94909d21dbf033cbe34089782c430ec25b9ad0bc
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] pylint fixes
On 06/20/2016 07:08 PM, Martin Basti wrote:
On 20.06.2016 19:06, Martin Basti wrote:
On 20.06.2016 12:00, Florence Blanc-Renaud wrote:
On 06/09/2016 05:10 PM, Petr Spacek wrote:
Hello,
I've received a bunch of pylint fixes produced by upstream contributor who is
not subscribed to the list so I'm resending them here.
All credit goes to Bárta Jan <55042ba...@sstebrno.eu>.
Flo, if you have time for it I think that it could be a good exercise which
will lead you to various dark corners in IPA :-)
Petr^2 Spacek
Forwarded Message
Date: Fri, 3 Jun 2016 14:57:16 +0200
From: Bárta Jan <55042ba...@sstebrno.eu>
To: pspa...@redhat.com
___- In the patch
0002-pylint-fix-simplifiable-if-statement-warnings.patch:_
diff --git a/ipatests/test_integration/tasks.py
b/ipatests/test_integration/tasks.py
index aebd907..ca2e10f 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -149,11 +149,7 @@ def host_service_active(host, service):
res = host.run_command(['systemctl', 'is-active', '--quiet',
service],
raiseonerr=False)
-if res.returncode == 0:
-return True
-else:
-return False
-
+return res.returncode
should be instead: return res.returncode *== 0* (otherwise the return
type is an int and not a boolean).
In the same file:
@@ -295,11 +291,7 @@ def
master_authoritative_for_client_domain(master, client):
zone = ".".join(client.hostname.split('.')[1:])
result = master.run_command(["ipa", "dnszone-show", zone],
raiseonerr=False)
-if result.returncode == 0:
-return True
-else:
-return False
-
+result.returncode == 0
should be instead: *return* result.returncode == 0 (otherwise there
is no return statement)
diff --git a/ipaserver/plugins/dogtag.py b/ipaserver/plugins/dogtag.py
index 197814c..36b6ba5 100644
--- a/ipaserver/plugins/dogtag.py
+++ b/ipaserver/plugins/dogtag.py
@@ -1689,12 +1689,7 @@ class ra(rabase.rabase):
# Return command result
cmd_result = {}
-if parse_result.get('revoked') == 'yes':
-cmd_result['revoked'] = True
-else:
-cmd_result['revoked'] = False
-
-return cmd_result
+cmd_result['revoked'] = parse_result.get('revoked')
Should be instead: cmd_result['revoked'] =
parse_result.get('revoked') *== 'yes'* (otherwise the type is a
string and not a boolean)
_- in the patch 00__04-pylint-fix-unneeded-not.patch_
@@ -632,7 +632,7 @@ class host_add(LDAPCreate):
options['ip_address'],
check_forward=True,
check_reverse=check_reverse)
-if not options.get('force', False) and not 'ip_address' in
options:
+if options.get('force', False) and 'ip_address' not in options:
Should be instead: if *not* options.get('force', False) and
'ip_address' not in options:
because of operators precedence
I will review patches 0005 to 0010 later today.
Flo.
How about patches 1, and 3? Because patches are independent, we can
separately ACK them and push them.
Martin^2
Sorry, I just noticed that there is no patch 1 :)
Patch 0003 is OK, ACK for this one.
Flo.
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] pylint fixes
On 20.06.2016 19:06, Martin Basti wrote:
On 20.06.2016 12:00, Florence Blanc-Renaud wrote:
On 06/09/2016 05:10 PM, Petr Spacek wrote:
Hello,
I've received a bunch of pylint fixes produced by upstream contributor who is
not subscribed to the list so I'm resending them here.
All credit goes to Bárta Jan<55042ba...@sstebrno.eu>.
Flo, if you have time for it I think that it could be a good exercise which
will lead you to various dark corners in IPA :-)
Petr^2 Spacek
Forwarded Message
Date: Fri, 3 Jun 2016 14:57:16 +0200
From: Bárta Jan<55042ba...@sstebrno.eu>
To:pspa...@redhat.com
___- In the patch
0002-pylint-fix-simplifiable-if-statement-warnings.patch:_
diff --git a/ipatests/test_integration/tasks.py
b/ipatests/test_integration/tasks.py
index aebd907..ca2e10f 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -149,11 +149,7 @@ def host_service_active(host, service):
res = host.run_command(['systemctl', 'is-active', '--quiet',
service],
raiseonerr=False)
-if res.returncode == 0:
-return True
-else:
-return False
-
+return res.returncode
should be instead: return res.returncode *== 0* (otherwise the return
type is an int and not a boolean).
In the same file:
@@ -295,11 +291,7 @@ def
master_authoritative_for_client_domain(master, client):
zone = ".".join(client.hostname.split('.')[1:])
result = master.run_command(["ipa", "dnszone-show", zone],
raiseonerr=False)
-if result.returncode == 0:
-return True
-else:
-return False
-
+result.returncode == 0
should be instead: *return* result.returncode == 0 (otherwise there
is no return statement)
diff --git a/ipaserver/plugins/dogtag.py b/ipaserver/plugins/dogtag.py
index 197814c..36b6ba5 100644
--- a/ipaserver/plugins/dogtag.py
+++ b/ipaserver/plugins/dogtag.py
@@ -1689,12 +1689,7 @@ class ra(rabase.rabase):
# Return command result
cmd_result = {}
-if parse_result.get('revoked') == 'yes':
-cmd_result['revoked'] = True
-else:
-cmd_result['revoked'] = False
-
-return cmd_result
+cmd_result['revoked'] = parse_result.get('revoked')
Should be instead: cmd_result['revoked'] =
parse_result.get('revoked') *== 'yes'* (otherwise the type is a
string and not a boolean)
_- in the patch 00__04-pylint-fix-unneeded-not.patch_
@@ -632,7 +632,7 @@ class host_add(LDAPCreate):
options['ip_address'],
check_forward=True,
check_reverse=check_reverse)
-if not options.get('force', False) and not 'ip_address' in
options:
+if options.get('force', False) and 'ip_address' not in options:
Should be instead: if *not* options.get('force', False) and
'ip_address' not in options:
because of operators precedence
I will review patches 0005 to 0010 later today.
Flo.
How about patches 1, and 3? Because patches are independent, we can
separately ACK them and push them.
Martin^2
Sorry, I just noticed that there is no patch 1 :)
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] pylint fixes
On 20.06.2016 12:00, Florence Blanc-Renaud wrote:
On 06/09/2016 05:10 PM, Petr Spacek wrote:
Hello,
I've received a bunch of pylint fixes produced by upstream contributor who is
not subscribed to the list so I'm resending them here.
All credit goes to Bárta Jan<55042ba...@sstebrno.eu>.
Flo, if you have time for it I think that it could be a good exercise which
will lead you to various dark corners in IPA :-)
Petr^2 Spacek
Forwarded Message
Date: Fri, 3 Jun 2016 14:57:16 +0200
From: Bárta Jan<55042ba...@sstebrno.eu>
To:pspa...@redhat.com
___- In the patch
0002-pylint-fix-simplifiable-if-statement-warnings.patch:_
diff --git a/ipatests/test_integration/tasks.py
b/ipatests/test_integration/tasks.py
index aebd907..ca2e10f 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -149,11 +149,7 @@ def host_service_active(host, service):
res = host.run_command(['systemctl', 'is-active', '--quiet',
service],
raiseonerr=False)
-if res.returncode == 0:
-return True
-else:
-return False
-
+return res.returncode
should be instead: return res.returncode *== 0* (otherwise the return
type is an int and not a boolean).
In the same file:
@@ -295,11 +291,7 @@ def
master_authoritative_for_client_domain(master, client):
zone = ".".join(client.hostname.split('.')[1:])
result = master.run_command(["ipa", "dnszone-show", zone],
raiseonerr=False)
-if result.returncode == 0:
-return True
-else:
-return False
-
+result.returncode == 0
should be instead: *return* result.returncode == 0 (otherwise there is
no return statement)
diff --git a/ipaserver/plugins/dogtag.py b/ipaserver/plugins/dogtag.py
index 197814c..36b6ba5 100644
--- a/ipaserver/plugins/dogtag.py
+++ b/ipaserver/plugins/dogtag.py
@@ -1689,12 +1689,7 @@ class ra(rabase.rabase):
# Return command result
cmd_result = {}
-if parse_result.get('revoked') == 'yes':
-cmd_result['revoked'] = True
-else:
-cmd_result['revoked'] = False
-
-return cmd_result
+cmd_result['revoked'] = parse_result.get('revoked')
Should be instead: cmd_result['revoked'] = parse_result.get('revoked')
*== 'yes'* (otherwise the type is a string and not a boolean)
_- in the patch 00__04-pylint-fix-unneeded-not.patch_
@@ -632,7 +632,7 @@ class host_add(LDAPCreate):
options['ip_address'],
check_forward=True,
check_reverse=check_reverse)
-if not options.get('force', False) and not 'ip_address' in
options:
+if options.get('force', False) and 'ip_address' not in options:
Should be instead: if *not* options.get('force', False) and
'ip_address' not in options:
because of operators precedence
I will review patches 0005 to 0010 later today.
Flo.
How about patches 1, and 3? Because patches are independent, we can
separately ACK them and push them.
Martin^2
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] pylint fixes
On 06/09/2016 05:10 PM, Petr Spacek wrote:
Hello,
I've received a bunch of pylint fixes produced by upstream contributor who is
not subscribed to the list so I'm resending them here.
All credit goes to Bárta Jan<55042ba...@sstebrno.eu>.
Flo, if you have time for it I think that it could be a good exercise which
will lead you to various dark corners in IPA :-)
Petr^2 Spacek
Forwarded Message
Date: Fri, 3 Jun 2016 14:57:16 +0200
From: Bárta Jan<55042ba...@sstebrno.eu>
To:pspa...@redhat.com
___- In the patch 0002-pylint-fix-simplifiable-if-statement-warnings.patch:_
diff --git a/ipatests/test_integration/tasks.py
b/ipatests/test_integration/tasks.py
index aebd907..ca2e10f 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -149,11 +149,7 @@ def host_service_active(host, service):
res = host.run_command(['systemctl', 'is-active', '--quiet', service],
raiseonerr=False)
-if res.returncode == 0:
-return True
-else:
-return False
-
+return res.returncode
should be instead: return res.returncode *== 0* (otherwise the return
type is an int and not a boolean).
In the same file:
@@ -295,11 +291,7 @@ def master_authoritative_for_client_domain(master,
client):
zone = ".".join(client.hostname.split('.')[1:])
result = master.run_command(["ipa", "dnszone-show", zone],
raiseonerr=False)
-if result.returncode == 0:
-return True
-else:
-return False
-
+result.returncode == 0
should be instead: *return* result.returncode == 0 (otherwise there is
no return statement)
diff --git a/ipaserver/plugins/dogtag.py b/ipaserver/plugins/dogtag.py
index 197814c..36b6ba5 100644
--- a/ipaserver/plugins/dogtag.py
+++ b/ipaserver/plugins/dogtag.py
@@ -1689,12 +1689,7 @@ class ra(rabase.rabase):
# Return command result
cmd_result = {}
-if parse_result.get('revoked') == 'yes':
-cmd_result['revoked'] = True
-else:
-cmd_result['revoked'] = False
-
-return cmd_result
+cmd_result['revoked'] = parse_result.get('revoked')
Should be instead: cmd_result['revoked'] = parse_result.get('revoked')
*== 'yes'* (otherwise the type is a string and not a boolean)
_- in the patch 00__04-pylint-fix-unneeded-not.patch_
@@ -632,7 +632,7 @@ class host_add(LDAPCreate):
options['ip_address'],
check_forward=True,
check_reverse=check_reverse)
-if not options.get('force', False) and not 'ip_address' in options:
+if options.get('force', False) and 'ip_address' not in options:
Should be instead: if *not* options.get('force', False) and 'ip_address'
not in options:
because of operators precedence
I will review patches 0005 to 0010 later today.
Flo.
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [PATCH] pylint fixes
Hello, I've received a bunch of pylint fixes produced by upstream contributor who is not subscribed to the list so I'm resending them here. All credit goes to Bárta Jan <55042ba...@sstebrno.eu>. Flo, if you have time for it I think that it could be a good exercise which will lead you to various dark corners in IPA :-) Petr^2 Spacek Forwarded Message Date: Fri, 3 Jun 2016 14:57:16 +0200 From: Bárta Jan <55042ba...@sstebrno.eu> To: pspa...@redhat.com From db04583b41c460e798e664379bc79f30e4aeb558 Mon Sep 17 00:00:00 2001 From: Jan Barta <55042ba...@sstebrno.eu> Date: Thu, 2 Jun 2016 09:58:52 +0200 Subject: [PATCH 02/10] pylint: fix simplifiable-if-statement warnings fix inefficient if statements, enable pylint check --- ipalib/config.py | 8 ++-- ipaplatform/base/services.py | 12 ipapython/ipautil.py | 10 ++ ipapython/nsslib.py| 5 + ipapython/sysrestore.py| 5 + ipaserver/install/ca.py| 7 +-- ipaserver/plugins/dogtag.py| 12 ++-- ipatests/test_integration/tasks.py | 12 ++-- pylintrc | 1 - 9 files changed, 15 insertions(+), 57 deletions(-) diff --git a/ipalib/config.py b/ipalib/config.py index 44dedf2..569e2f7 100644 --- a/ipalib/config.py +++ b/ipalib/config.py @@ -455,14 +455,10 @@ class Env(object): # Determine if running in source tree: if 'in_tree' not in self: -if ( +self.in_tree = ( self.bin == self.site_packages and path.isfile(path.join(self.bin, 'setup.py')) -): -self.in_tree = True -else: -self.in_tree = False - +) if self.in_tree and 'mode' not in self: self.mode = 'developer' diff --git a/ipaplatform/base/services.py b/ipaplatform/base/services.py index 641a654..928fe76 100644 --- a/ipaplatform/base/services.py +++ b/ipaplatform/base/services.py @@ -271,10 +271,8 @@ class SystemdService(PlatformService): ipautil.run(args, skip_output=not capture_output) -if getattr(self.api.env, 'context', None) in ['ipactl', 'installer']: -update_service_list = True -else: -update_service_list = False +update_service_list = getattr(self.api.env, 'context', + None) in ['ipactl', 'installer'] super(SystemdService, self).stop( instance_name, update_service_list=update_service_list) @@ -284,10 +282,8 @@ class SystemdService(PlatformService): self.service_instance(instance_name)], skip_output=not capture_output) -if getattr(self.api.env, 'context', None) in ['ipactl', 'installer']: -update_service_list = True -else: -update_service_list = False +update_service_list = getattr(self.api.env, 'context', + None) in ['ipactl', 'installer'] if wait and self.is_running(instance_name): self.wait_for_open_ports(self.service_instance(instance_name)) diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py index 34e05d3..fd24c89 100644 --- a/ipapython/ipautil.py +++ b/ipapython/ipautil.py @@ -489,20 +489,14 @@ def nolog_replace(string, nolog): def file_exists(filename): try: mode = os.stat(filename)[stat.ST_MODE] -if stat.S_ISREG(mode): -return True -else: -return False +return bool(stat.S_ISREG(mode)) except Exception: return False def dir_exists(filename): try: mode = os.stat(filename)[stat.ST_MODE] -if stat.S_ISDIR(mode): -return True -else: -return False +return bool(stat.S_ISDIR(mode)) except Exception: return False diff --git a/ipapython/nsslib.py b/ipapython/nsslib.py index b5e5b65..1573de9 100644 --- a/ipapython/nsslib.py +++ b/ipapython/nsslib.py @@ -74,10 +74,7 @@ def auth_certificate_callback(sock, check_sig, is_server, certdb): ', '.join(nss.cert_usage_flags(intended_usage))) # Is the intended usage a proper subset of the approved usage -if approved_usage & intended_usage: -cert_is_valid = True -else: -cert_is_valid = False +cert_is_valid = bool(approved_usage & intended_usage) # If this is a server, we're finished if is_server or not cert_is_valid: diff --git a/ipapython/sysrestore.py b/ipapython/sysrestore.py index e0d0908..cd09cae 100644 --- a/ipapython/sysrestore.py +++ b/ipapython/sysrestore.py @@ -437,7 +437,4 @@ class StateFile: Can be used to determine if a service is configured. """ -if module in self.modules: -return True -else: -return False +return module in se
