Re: [Freeipa-devel] [PATCH] 0037 make-lint fails on Fedora 16/Rawhide
On Mon, 2011-11-28 at 17:26 +0200, Alexander Bokovoy wrote: On Mon, 28 Nov 2011, Alexander Bokovoy wrote: Hi, Attached are fixes for ldap.LDAPObject.add_s(self, dn, modlist) uses which now don't pass 'make-lint' on Fedora 16/Rawhide. -- / Alexander Bokovoy From dd866262c98be779a094a617975145e2fb1e0dd1 Mon Sep 17 00:00:00 2001 From: Alexander Bokovoy aboko...@redhat.com Date: Mon, 28 Nov 2011 14:21:17 +0200 Subject: [PATCH] Be more explicit when passing Entry class to ldap.LDAPObject.add_s() ldap.LDAPObject.add_s(self, dn, modlist) requires two positional arguments. We used to pass our Entry class which implements dictionary access that gives proper way to pass the positional arguments, but PyLint in Fedora 16/Rawhide became more strict about that and can't infer dictionary interface through static checking. Thus, we need to explicitly annotate dictionary passing with **entry syntax. This has additional benefit to remind that we deal with multiple arguments here. Self-NACK, doesn't really work this way. I'm still looking at better approach but intermediate solution is to use pylint hints. Conservative patch is attached. What about other add_s(entry) calls? I see we call it this way on more places, especially in replication.py: $ git grep add_s(e ipaserver/install/cainstance.py:ld.add_s(entry_dn, entry) ipaserver/install/krbinstance.py:self.admin_conn.add_s(entry) #pylint: disable=E1120 ipaserver/install/krbinstance.py:self.admin_conn.add_s(entry) #pylint: disable=E1120 ipaserver/install/replication.py:conn.add_s(ent) ipaserver/install/replication.py:conn.add_s(entry) ipaserver/install/replication.py:conn.add_s(entry) ipaserver/install/replication.py:self.conn.add_s(entry) ipaserver/install/replication.py:conn.add_s(entry) ipaserver/install/replication.py:a_conn.add_s(entry) ipaserver/install/replication.py:self.conn.add_s(entry) ipaserver/install/service.py:conn.add_s(entry) #pylint: disable=E1120 Should we patch ipa-2-1 branch as well? If we do another release for F-16 we want to have pylint check clean. We would need a rebased patch for ipa-2-1 branch in this case. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0037 make-lint fails on Fedora 16/Rawhide
On Tue, 29 Nov 2011, Martin Kosek wrote: Conservative patch is attached. What about other add_s(entry) calls? I see we call it this way on more places, especially in replication.py: $ git grep add_s(e ipaserver/install/cainstance.py:ld.add_s(entry_dn, entry) ipaserver/install/krbinstance.py:self.admin_conn.add_s(entry) #pylint: disable=E1120 ipaserver/install/krbinstance.py:self.admin_conn.add_s(entry) #pylint: disable=E1120 ipaserver/install/replication.py:conn.add_s(ent) ipaserver/install/replication.py:conn.add_s(entry) ipaserver/install/replication.py:conn.add_s(entry) ipaserver/install/replication.py:self.conn.add_s(entry) ipaserver/install/replication.py:conn.add_s(entry) ipaserver/install/replication.py:a_conn.add_s(entry) ipaserver/install/replication.py:self.conn.add_s(entry) ipaserver/install/service.py:conn.add_s(entry) #pylint: disable=E1120 Should we patch ipa-2-1 branch as well? If we do another release for F-16 we want to have pylint check clean. We would need a rebased patch for ipa-2-1 branch in this case. I think this patch also can go away, we found out with Rob yesterday night that John has addressed these issues in his patches 48 and 49 in September. The patches were ACKed but not merged. I'm doing their testing now and will push them after that. Look at John's 48/49 patches in meanwhile. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0037 make-lint fails on Fedora 16/Rawhide
On Tue, 2011-11-29 at 13:54 +0200, Alexander Bokovoy wrote: On Tue, 29 Nov 2011, Alexander Bokovoy wrote: On Tue, 29 Nov 2011, Martin Kosek wrote: Conservative patch is attached. What about other add_s(entry) calls? I see we call it this way on more places, especially in replication.py: $ git grep add_s(e ipaserver/install/cainstance.py:ld.add_s(entry_dn, entry) ipaserver/install/krbinstance.py:self.admin_conn.add_s(entry) #pylint: disable=E1120 ipaserver/install/krbinstance.py:self.admin_conn.add_s(entry) #pylint: disable=E1120 ipaserver/install/replication.py:conn.add_s(ent) ipaserver/install/replication.py:conn.add_s(entry) ipaserver/install/replication.py:conn.add_s(entry) ipaserver/install/replication.py:self.conn.add_s(entry) ipaserver/install/replication.py:conn.add_s(entry) ipaserver/install/replication.py:a_conn.add_s(entry) ipaserver/install/replication.py:self.conn.add_s(entry) ipaserver/install/service.py:conn.add_s(entry) #pylint: disable=E1120 Should we patch ipa-2-1 branch as well? If we do another release for F-16 we want to have pylint check clean. We would need a rebased patch for ipa-2-1 branch in this case. I think this patch also can go away, we found out with Rob yesterday night that John has addressed these issues in his patches 48 and 49 in September. The patches were ACKed but not merged. I'm doing their testing now and will push them after that. Look at John's 48/49 patches in meanwhile. Ok, I've tested patches 48 and 49 and also made versions for ipa-2-1 branch. All attached. Both pushed to master. As discussed on IRC, we are not sure that these pylint errors would manifest in ipa-2-1 branch, so for the sake of stability I decided to push them to master branch only. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0037 make-lint fails on Fedora 16/Rawhide
On Mon, 28 Nov 2011, Alexander Bokovoy wrote: Hi, Attached are fixes for ldap.LDAPObject.add_s(self, dn, modlist) uses which now don't pass 'make-lint' on Fedora 16/Rawhide. -- / Alexander Bokovoy From dd866262c98be779a094a617975145e2fb1e0dd1 Mon Sep 17 00:00:00 2001 From: Alexander Bokovoy aboko...@redhat.com Date: Mon, 28 Nov 2011 14:21:17 +0200 Subject: [PATCH] Be more explicit when passing Entry class to ldap.LDAPObject.add_s() ldap.LDAPObject.add_s(self, dn, modlist) requires two positional arguments. We used to pass our Entry class which implements dictionary access that gives proper way to pass the positional arguments, but PyLint in Fedora 16/Rawhide became more strict about that and can't infer dictionary interface through static checking. Thus, we need to explicitly annotate dictionary passing with **entry syntax. This has additional benefit to remind that we deal with multiple arguments here. Self-NACK, doesn't really work this way. I'm still looking at better approach but intermediate solution is to use pylint hints. Conservative patch is attached. -- / Alexander Bokovoy From 4eb6c88392f43d57943426968a98fca1a91f302c Mon Sep 17 00:00:00 2001 From: Alexander Bokovoy aboko...@redhat.com Date: Mon, 28 Nov 2011 17:02:11 +0200 Subject: [PATCH] Disable pylint false positive when passing Entry class to ldap.LDAPObject.add_s() ldap.LDAPObject.add_s(self, dn, modlist) requires two positional arguments. We used to pass our Entry class which implements dictionary access that gives proper way to pass the positional arguments, but PyLint in Fedora 16/Rawhide became more strict about that and can't infer dictionary interface through static checking. Thus, we need to explicitly mark this error as false positive as PyLint is unable to guess that. This has additional benefit to remind that we deal with multiple arguments here. --- ipaserver/install/adtrustinstance.py |6 +++--- ipaserver/install/krbinstance.py |4 ++-- ipaserver/install/service.py |2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py index 96f99dc9b76370c07b727e8e94ccabfe7afca074..8a9e3d9375f00fef9eb7aafb0a22e88cd09d7270 100644 --- a/ipaserver/install/adtrustinstance.py +++ b/ipaserver/install/adtrustinstance.py @@ -99,7 +99,7 @@ class ADTRUSTInstance(service.Service): entry.setValues(objectclass, [account, simplesecurityobject]) entry.setValues(uid, samba) entry.setValues(userPassword, self.smb_dn_pwd) -self.admin_conn.add_s(entry) +self.admin_conn.add_s(entry) #pylint: disable=E1120 # And finally grant it permission to read NT passwords, we do not want # to support LM passwords so there is no need to allow access to them. @@ -188,7 +188,7 @@ class ADTRUSTInstance(service.Service): entry = ipaldap.Entry(self.trust_dn) entry.setValues(objectclass, [nsContainer]) entry.setValues(cn, trusts) -self.admin_conn.add_s(entry) +self.admin_conn.add_s(entry) #pylint: disable=E1120 entry = ipaldap.Entry(self.smb_dom_dn) entry.setValues(objectclass, [sambaDomain, nsContainer]) @@ -196,7 +196,7 @@ class ADTRUSTInstance(service.Service): entry.setValues(sambaDomainName, self.netbios_name) entry.setValues(sambaSID, self.__gen_sid_string()) #TODO: which MAY attributes do we want to set ? -self.admin_conn.add_s(entry) +self.admin_conn.add_s(entry) #pylint: disable=E1120 def __write_smb_conf(self): self.fstore.backup_file(self.smb_conf) diff --git a/ipaserver/install/krbinstance.py b/ipaserver/install/krbinstance.py index 6ed38516223a56e723bcc9cf643ac2b362200da4..193dacd2b406332c83ed8b3b86cd7b3cff20471a 100644 --- a/ipaserver/install/krbinstance.py +++ b/ipaserver/install/krbinstance.py @@ -259,7 +259,7 @@ class KrbInstance(service.Service): entry.setValues(nsSaslMapFilterTemplate, '(krbPrincipalName=\\1@\\2)') try: -self.admin_conn.add_s(entry) +self.admin_conn.add_s(entry) #pylint: disable=E1120 except ldap.ALREADY_EXISTS: root_logger.critical(failed to add Full Principal Sasl mapping) raise e @@ -272,7 +272,7 @@ class KrbInstance(service.Service): entry.setValues(nsSaslMapFilterTemplate, '(krbPrincipalName=@%s)' % self.realm) try: -self.admin_conn.add_s(entry) +self.admin_conn.add_s(entry) #pylint: disable=E1120 except ldap.ALREADY_EXISTS: root_logger.critical(failed to add Name Only Sasl mapping) raise e diff --git a/ipaserver/install/service.py b/ipaserver/install/service.py index 249727b15b6f3c879336c36d43410fc7234d9d9d..6bc70fa2e661a01b78a0a74490c56dabfce5 100644 --- a/ipaserver/install/service.py +++ b/ipaserver/install/service.py
