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 <[email protected]>
To: Mathieu Bridon <[email protected]>
CC: Pavol Babincak <[email protected]>
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
From 7e4816833ace17d780da2cf7d5901f610163d3a4 Mon Sep 17 00:00:00 2001
From: Mike Bonnet <[email protected]>
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
--
buildsys mailing list
[email protected]
https://admin.fedoraproject.org/mailman/listinfo/buildsys
