Fwd: Re: rebased rpkg patches
Pavol Babincak
pbabinca at redhat.com
Thu Dec 4 10:21:58 UTC 2014
Because Mike's original runas patch broke fedpkg (and possible other
pyrpkg clients) I asked him to send fix for that. He haven't sent it to
buildsys mailing list, so forwarding it here. Patch is applied and
pushed upstream now.
-------- Forwarded Message --------
Subject: Re: rebased rpkg patches
Date: Wed, 03 Dec 2014 11:42:43 -0800
From: Mike Bonnet <mikeb at redhat.com>
To: Mathieu Bridon <mbridon at redhat.com>
CC: Pavol Babincak <pbabinca at redhat.com>
On 11/12/14 10:19 AM, Mike Bonnet wrote:
> On 11/4/14 5:03 AM, Mike Bonnet wrote:
>> On 11/04/2014 06:08 AM, Mathieu Bridon wrote:
>>> On Mon, 2014-11-03 at 15:56 -0500, Mike Bonnet wrote:
>>>> On 11/03/2014 04:11 AM, Mathieu Bridon wrote:
>>>>> On Mon, 2014-11-03 at 10:10 +0100, Mathieu Bridon wrote:
>>>>>> On Fri, 2014-10-31 at 09:07 -0700, Mike Bonnet wrote:
>>>>>> def __init__(self, path, lookaside, lookasidehash,
>>>>>> lookaside_cgi,
>>>>>> gitbaseurl, anongiturl, branchre, kojiconfig,
>>>>>> - build_client, user=None, dist=None, target=None,
>>>>>> + build_client, user=None, runas=None, dist=None,
>>>>>> target=None,
>>>>>>
>>>>>> This breaks fedpkg.
>>>>>>
>>>>>> To be honest, it is fedpkg that is doing something wrong here:
>>>>>>
>>>>>> super(Commands, self).__init__(path, lookaside,
>>>>>> lookasidehash,
>>>>>> lookaside_cgi, gitbaseurl,
>>>>>> anongiturl,
>>>>>> branchre, kojiconfig,
>>>>>> build_client,
>>>>>> user, dist, target, quiet)
>>>>>>
>>>>>> Keyword arguments passed as positional arguments. :(
>>>>>>
>>>>>> However, it means that applying your patch to rpkg as it is will
>>>>>> break
>>>>>> fedpkg, and potentially other consumers of the pyrpkg API using it in
>>>>>> the same (wrong) way.
>>>>>>
>>>>>> At the very least, we need to fix fedpkg so it correctly uses the
>>>>>> pyrpkg
>>>>>> API (passing kwargs as kwargs).
>>>>>>
>>>>>> Maybe we should also announce it widely that this change is coming,
>>>>>> and
>>>>>> people should check their code to make sure they don't do the same
>>>>>> thing
>>>>>> as fedpkg?
>>>>>
>>>>> Also, it would have been nicer to send these patches to the public
>>>>> mailing-list, as the discussion about how this breaks fedpkg could
>>>>> have
>>>>> served as a starting point for widely announcing that it might break
>>>>> other consumers. :)
>>>>
>>>> I did send a message about the original (pre-rebase) patch set to
>>>> Fedora
>>>> buildsys list.
>>>>
>>>> Would it be better to move runas=None to the end of the keyword
>>>> parameter list of __init__()? This should avoid the breakage of
>>>> fedpkg.
>>>
>>> It should. (and password=None as well)
>>
>> I'll send a fixed-up patch to Fedora buildsys list.
>
> I actually sent a patch to fix fedpkg to buildsys list. Could we get
> that applied and rolled out, then go with the rpkg patches as-is?
The attached patch fixes the fedpkg breakage. It handles the new
command-line options the same way the module_name parameter is handled.
--
Pavol Babincak
Release Engineering, Red Hat
-------------- next part --------------
From 7e4816833ace17d780da2cf7d5901f610163d3a4 Mon Sep 17 00:00:00 2001
From: Mike Bonnet <mikeb at redhat.com>
Date: Wed, 3 Dec 2014 11:37:46 -0800
Subject: [PATCH] pass extra data to the Commands object via properties instead
of __init__()
Subclasses of the Commands object may not handle new keyword arguments to __init__(). Pass the new password and runas
command-line options via properties instead, which is compatible with subclassing.
---
src/pyrpkg/__init__.py | 14 +++++++++++---
src/pyrpkg/cli.py | 4 ++--
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/src/pyrpkg/__init__.py b/src/pyrpkg/__init__.py
index a37a11b..5c4dbc5 100644
--- a/src/pyrpkg/__init__.py
+++ b/src/pyrpkg/__init__.py
@@ -73,7 +73,7 @@ class Commands(object):
def __init__(self, path, lookaside, lookasidehash, lookaside_cgi,
gitbaseurl, anongiturl, branchre, kojiconfig,
- build_client, user=None, password=None, runas=None,
+ build_client, user=None,
dist=None, target=None, quiet=False):
"""Init the object and some configuration details."""
@@ -146,9 +146,9 @@ class Commands(object):
# The user to use or discover
self._user = user
# The password to use
- self._password = password
+ self._password = None
# The alternate Koji user to run commands as
- self._runas = runas
+ self._runas = None
# The rpm version of the cloned module
self._ver = None
self.log = log
@@ -691,12 +691,20 @@ class Commands(object):
return self._password
+ @password.setter
+ def password(self, password):
+ self._password = password
+
@property
def runas(self):
"""This property ensures the runas attribute"""
return self._runas
+ @runas.setter
+ def runas(self, runas):
+ self._runas = runas
+
@property
def ver(self):
"""This property ensures the ver attribute"""
diff --git a/src/pyrpkg/cli.py b/src/pyrpkg/cli.py
index 5afaf16..bc4bc90 100755
--- a/src/pyrpkg/cli.py
+++ b/src/pyrpkg/cli.py
@@ -105,14 +105,14 @@ class cliClient(object):
items['kojiconfig'],
items['build_client'],
user=self.args.user,
- password=self.args.password,
- runas=self.args.runas,
dist=self.args.dist,
target=target,
quiet=self.args.q,
)
self._cmd.module_name = self.args.module_name
+ self._cmd.password = self.args.password
+ self._cmd.runas = self.args.runas
# This function loads the extra stuff once we figure out what site
# we are
--
1.9.3
More information about the buildsys
mailing list