Re: [Spacewalk-devel] [PATCH] make reposync more modular

2011-12-05 Thread Ionuț Arțăriși 

On 12/02/2011 05:36 PM, Jan Pazdziora wrote:

On Fri, Dec 02, 2011 at 05:23:25PM +0100, Ionuț Arțăriși wrote:

Hello,

I've written a patch to make reposync more modular. The essential
thing is moving the argument parsing to the spacewalk-repo-sync
script and out of python/site-packages, but there are also a lot of
other style improvements.

We've already applied those changes and others in our branch, I just
merged some of them into the spacewalk branch now.

Can you post this as multiple patches, one for each type of change?
This will make it easier to review than this one huge patch.


Ok, I've chopped that patch up. Here are the 18 resulting patches.


Thanks,
-Ionuț
>From 510fd29c84d1684d9ecbf4e25a5e35a6a23c86f9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ionu=C8=9B=20Ar=C8=9B=C4=83ri=C8=99i?= 
Date: Mon, 5 Dec 2011 13:57:05 +0100
Subject: [PATCH 15/18] word-wrap to <80 chars and fix string concatenation

---
 backend/satellite_tools/repo_plugins/yum_src.py |   12 
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/backend/satellite_tools/repo_plugins/yum_src.py b/backend/satellite_tools/repo_plugins/yum_src.py
index dff98c7..34212c4 100644
--- a/backend/satellite_tools/repo_plugins/yum_src.py
+++ b/backend/satellite_tools/repo_plugins/yum_src.py
@@ -89,12 +89,16 @@ class ContentSource:
 self.proxy_user = CFG.http_proxy_username
 self.proxy_pass = CFG.http_proxy_password
 
-if (self.proxy_user is not None and self.proxy_pass is not None and self.proxy_addr is not None):
-self.proxy_url = "http://%s:%s@%s"; %(self.proxy_user, self.proxy_pass, self.proxy_addr)
-elif (self.proxy_addr is not None):
-self.proxy_url = "http://%s"; %(self.proxy_addr)
+if (self.proxy_user is not None and
+self.proxy_pass is not None and
+self.proxy_addr is not None):
+self.proxy_url = "http://%s:%s@%s"; % (
+self.proxy_user, self.proxy_pass, self.proxy_addr)
+elif self.proxy_addr is not None:
+self.proxy_url = "http://"; + self.proxy_addr
 else:
 self.proxy_url = None
+
 repo = yum.yumRepo.YumRepository(name)
 self.repo = repo
 repo.cache = 0
-- 
1.7.3.4

>From 52397d165c922ebc0551b3744770039a8f5739c7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ionu=C8=9B=20Ar=C8=9B=C4=83ri=C8=99i?= 
Date: Mon, 5 Dec 2011 13:54:43 +0100
Subject: [PATCH 14/18] CACHE_DIR is a constant so we declare it at the top of the file

---
 backend/satellite_tools/repo_plugins/yum_src.py |   10 ++
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/backend/satellite_tools/repo_plugins/yum_src.py b/backend/satellite_tools/repo_plugins/yum_src.py
index 1b85b79..dff98c7 100644
--- a/backend/satellite_tools/repo_plugins/yum_src.py
+++ b/backend/satellite_tools/repo_plugins/yum_src.py
@@ -32,6 +32,8 @@ except ImportError:
 from spacewalk.satellite_tools.reposync import ContentPackage
 from spacewalk.common.rhnConfig import CFG, initCFG
 
+CACHE_DIR = '/var/cache/rhn/reposync/'
+
 class YumWarnings:
 def write(self, s):
 pass
@@ -78,9 +80,8 @@ class YumUpdateMetadata(UpdateMetadata):
 
 class ContentSource:
 repo = None
-cache_dir = '/var/cache/rhn/reposync/'
 def __init__(self, url, name):
-self._clean_cache(self.cache_dir + name)
+self._clean_cache(CACHE_DIR + name)
 
 # read the proxy configuration in /etc/rhn/rhn.conf
 initCFG('server.satellite')
@@ -100,9 +101,10 @@ class ContentSource:
 repo.metadata_expire = 0
 repo.mirrorlist = url
 repo.baseurl = [url]
-repo.basecachedir = self.cache_dir
+repo.basecachedir = CACHE_DIR
 if hasattr(repo, 'base_persistdir'):
-repo.base_persistdir = self.cache_dir
+repo.base_persistdir = CACHE_DIR
+
 if self.proxy_url is not None:
 repo.proxy = self.proxy_url
 
-- 
1.7.3.4

>From 0cb2f4cf338cc139b94ec60b44c4da8a46caced6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ionu=C8=9B=20Ar=C8=9B=C4=83ri=C8=99i?= 
Date: Mon, 5 Dec 2011 13:52:19 +0100
Subject: [PATCH 12/18] move third-party module import yum lower

---
 backend/satellite_tools/repo_plugins/yum_src.py |   10 ++
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/backend/satellite_tools/repo_plugins/yum_src.py b/backend/satellite_tools/repo_plugins/yum_src.py
index f0949da..47da5fc 100644
--- a/backend/satellite_tools/repo_plugins/yum_src.py
+++ b/backend/satellite_tools/repo_plugins/yum_src.py
@@ -1,6 +1,6 @@
 #
 # Copyright (c) 2008--2011 Red Hat, Inc.
-# Copyright (c) 2010--2011 SUSE Linux Products GmbH
+# Copyright (c) 2010--2011 SUSE LINUX Products GmbH, Nuernberg, Germany.
 #
 # This software is licensed to you under the GNU General Public License,
 # version 2 (GPLv2). There

[Spacewalk-devel] [PATCH] make reposync more modular

2011-12-02 Thread Ionuț Arțăriși 

Hello,

I've written a patch to make reposync more modular. The essential thing 
is moving the argument parsing to the spacewalk-repo-sync script and out 
of python/site-packages, but there are also a lot of other style 
improvements.


We've already applied those changes and others in our branch, I just 
merged some of them into the spacewalk branch now.


I hope you like it,
-Ionuț
>From e8a14d14817f57432c14c1d469bc07559359bf40 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ionu=C8=9B=20Ar=C8=9B=C4=83ri=C8=99i?= 
Date: Fri, 2 Dec 2011 17:09:13 +0100
Subject: [PATCH] make reposync more modular

- move argument parsing to the spacewalk-repo-sync script
- move repo setup to a special method in yum_src
- fixed other minor issues (indentation, class constructor, other pep-8 issues)
---
 backend/satellite_tools/repo_plugins/yum_src.py |   60 +
 backend/satellite_tools/reposync.py |  155 +--
 backend/satellite_tools/spacewalk-repo-sync |   64 +++---
 3 files changed, 142 insertions(+), 137 deletions(-)

diff --git a/backend/satellite_tools/repo_plugins/yum_src.py b/backend/satellite_tools/repo_plugins/yum_src.py
index f0949da..8d0a557 100644
--- a/backend/satellite_tools/repo_plugins/yum_src.py
+++ b/backend/satellite_tools/repo_plugins/yum_src.py
@@ -1,6 +1,6 @@
 #
 # Copyright (c) 2008--2011 Red Hat, Inc.
-# Copyright (c) 2010--2011 SUSE Linux Products GmbH
+# Copyright (c) 2010--2011 SUSE LINUX Products GmbH, Nuernberg, Germany.
 #
 # This software is licensed to you under the GNU General Public License,
 # version 2 (GPLv2). There is NO WARRANTY for this software, express or
@@ -8,15 +8,17 @@
 # FOR A PARTICULAR PURPOSE. You should have received a copy of GPLv2
 # along with this software; if not, see
 # http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
-# 
+#
 # Red Hat trademarks are not licensed under GPLv2. No permission is
 # granted to use or replicate Red Hat trademarks that are incorporated
-# in this software or its documentation. 
+# in this software or its documentation.
 #
-import yum
+
 import shutil
 import sys
 import gzip
+
+import yum
 from yum.update_md import UpdateMetadata, UpdateNoticeException, UpdateNotice
 from yum.yumRepo import YumRepository
 try:
@@ -30,6 +32,8 @@ except ImportError:
 from spacewalk.satellite_tools.reposync import ContentPackage
 from spacewalk.common.rhnConfig import CFG, initCFG
 
+CACHE_DIR = '/var/cache/rhn/reposync/'
+
 class YumWarnings:
 def write(self, s):
 pass
@@ -75,10 +79,10 @@ class YumUpdateMetadata(UpdateMetadata):
 no.add(un)
 
 class ContentSource:
-repo = None
-cache_dir = '/var/cache/rhn/reposync/'
 def __init__(self, url, name):
-self._clean_cache(self.cache_dir + name)
+self.url = url
+self.name = name
+self._clean_cache(CACHE_DIR + name)
 
 # read the proxy configuration in /etc/rhn/rhn.conf
 initCFG('server.satellite')
@@ -92,15 +96,25 @@ class ContentSource:
 self.proxy_url = "http://%s"; %(self.proxy_addr)
 else:
 self.proxy_url = None
+
 repo = yum.yumRepo.YumRepository(name)
 self.repo = repo
+self.sack = None
+
+self.setup_repo(repo)
+self.num_packages = 0
+self.num_excluded = 0
+
+def setup_repo(self, repo):
+"""Fetch repository metadata"""
 repo.cache = 0
 repo.metadata_expire = 0
-repo.mirrorlist = url
-repo.baseurl = [url]
-repo.basecachedir = self.cache_dir
+repo.mirrorlist = self.url
+repo.baseurl = [self.url]
+repo.basecachedir = CACHE_DIR
 if hasattr(repo, 'base_persistdir'):
-repo.base_persistdir = self.cache_dir
+repo.base_persistdir = CACHE_DIR
+
 if self.proxy_url is not None:
 repo.proxy = self.proxy_url
 
@@ -108,28 +122,24 @@ class ContentSource:
 warnings.disable()
 repo.baseurlSetup()
 warnings.restore()
-
 repo.setup(False)
-self.num_packages = 0
-self.num_excluded = 0
-
+self.sack = self.repo.getPackageSack()
 
 def list_packages(self, filters):
 """ list packages"""
-sack = self.repo.getPackageSack()
-sack.populate(self.repo, 'metadata', None, 0)
-list = sack.returnPackages()
+self.sack.populate(self.repo, 'metadata', None, 0)
+list = self.sack.returnPackages()
 self.num_packages = len(list)
 if filters:
 list = self._filter_packages(list, filters)
-list = self._get_package_dependencies(sack, list)
+list = self._get_package_dependencies(self.sack, list)
 self.num_excluded = self.num_packages - len(list)
 to_return = []
 for pack in list:
 if pack.arch == 'src':
 continue
 new_pack = ContentPackage()
-new_pack.setNVREA(pack.name, pack

Re: [Spacewalk-devel] erratas and the client plugin package action

2011-10-18 Thread Ionuț Arțăriși 

On 10/18/2011 05:17 PM, Miroslav Suchý wrote:

On 10/18/2011 04:53 PM, Ionuț Arțăriși wrote:

Can we still move errata.py to yum-rhn-plugin? We've gone with the
second option until now, but this would make packaging cleaner.

Still no objections.

Cool. So can you apply the patch that I attached to my previous email?

Thanks,
-Ionuț

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] erratas and the client plugin package action

2011-10-18 Thread Ionuț Arțăriși 

On 05/13/2011 02:27 PM, Miroslav Suchý wrote:

On 05/13/2011 02:22 PM, Duncan Mac-Vicar P. wrote:

Would it make more sense for errata.py to be in the yum-rhn-plugin?

It fine with me. yum-rhn-plugin and rhn-client-tools require each other
(on rhel/fedora) anyway. So yes, it can be moved to yum-rhn-plugin.


For *SUSE I think we will have to override errata.py as zypper should
install the patch directly and not let the plugin resolve the package list.

Would it be acceptable upstream that we don't install errata.py from the
.spec file %if 0%{?suse_version} and supply it with
zypp-plugin-spacewalk (which contains package.py)?

That possible as well.

I slightly prefer the first option (move it to yum-rhn-plugin). But
choose yourself.

Can we still move errata.py to yum-rhn-plugin? We've gone with the 
second option until now, but this would make packaging cleaner.


-Ionuț
>From 8bfecb11e2f14b1f922bb2986693840e8dc77107 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ionu=C8=9B=20Ar=C8=9B=C4=83ri=C8=99i?= 
Date: Tue, 18 Oct 2011 16:49:24 +0200
Subject: [PATCH] move errata.py action to the yum-rhn-plugin package

---
 client/rhel/rhn-client-tools/rhn-client-tools.spec |1 -
 client/rhel/rhn-client-tools/src/actions/Makefile  |2 +-
 client/rhel/rhn-client-tools/src/actions/errata.py |   87 
 client/rhel/yum-rhn-plugin/actions/errata.py   |   87 
 4 files changed, 88 insertions(+), 89 deletions(-)
 delete mode 100644 client/rhel/rhn-client-tools/src/actions/errata.py
 create mode 100644 client/rhel/yum-rhn-plugin/actions/errata.py

diff --git a/client/rhel/rhn-client-tools/rhn-client-tools.spec b/client/rhel/rhn-client-tools/rhn-client-tools.spec
index 8d9f58b..51292fb 100644
--- a/client/rhel/rhn-client-tools/rhn-client-tools.spec
+++ b/client/rhel/rhn-client-tools/rhn-client-tools.spec
@@ -249,7 +249,6 @@ make -f Makefile.rhn-client-tools test
 # actions for rhn_check to run
 %{_datadir}/rhn/actions/__init__.*
 %{_datadir}/rhn/actions/hardware.*
-%{_datadir}/rhn/actions/errata.*
 %{_datadir}/rhn/actions/systemid.*
 %{_datadir}/rhn/actions/reboot.*
 %{_datadir}/rhn/actions/rhnsd.*
diff --git a/client/rhel/rhn-client-tools/src/actions/Makefile b/client/rhel/rhn-client-tools/src/actions/Makefile
index 2d72848..fec4eff 100644
--- a/client/rhel/rhn-client-tools/src/actions/Makefile
+++ b/client/rhel/rhn-client-tools/src/actions/Makefile
@@ -3,7 +3,7 @@
 # $Id$
 
 ACTIONS		= up2date_config hardware reboot \
-		  rhnsd systemid errata __init__
+		  rhnsd systemid __init__
 PYFILES		= $(addsuffix .py, $(ACTIONS))
 OBJECTS		= $(PYFILES) $(addsuffix .pyc, $(ACTIONS))
 
diff --git a/client/rhel/rhn-client-tools/src/actions/errata.py b/client/rhel/rhn-client-tools/src/actions/errata.py
deleted file mode 100644
index 49f1c65..000
--- a/client/rhel/rhn-client-tools/src/actions/errata.py
+++ /dev/null
@@ -1,87 +0,0 @@
-#!/usr/bin/python
-
-# Client code for Update Agent
-# Copyright (c) 1999-2002 Red Hat, Inc.  Distributed under GPL.
-#
-# Author: Adrian Likins  4:
-# Newer sats send down arch, filter using name+arch
-for p in packagelist:
-	if current_packages_with_arch.has_key(p[0]+p[4]):
-	u[p[0]+p[4]] = p
-	elif current_packages_with_arch.has_key(p[0]+"noarch"):
-	u[p[0]+p[4]] = p
-	elif p[4] == "noarch" and current_packages.has_key(p[0]):
-	u[p[0]] = p
-else:
-# 5.2 and older sats + hosted dont send arch
-for p in packagelist:
-if current_packages.has_key(p[0]):
-u[p[0]] = p
-
-
-# XXX: Fix me - once we keep all errata packages around,
-# this is the WRONG thing to do - we want to keep the specific versions
-# that the user has asked for.
-packagelist = map(lambda a: u[a], u.keys())
-   
-if packagelist == []:
-	data = {}
-	data['version'] = "0"
-	data['name'] = "errata.update.no_packages"
-	data['erratas'] = errataidlist
-	
-	return (39, 
-		"No packages from that errata are available", 
-		data)
- 
-return packages.update(packagelist, cache_only)
-   
-
-def main():
-	print update([23423423])
-
-
-if __name__ == "__main__":
-	main()
- 
diff --git a/client/rhel/yum-rhn-plugin/actions/errata.py b/client/rhel/yum-rhn-plugin/actions/errata.py
new file mode 100644
index 000..49f1c65
--- /dev/null
+++ b/client/rhel/yum-rhn-plugin/actions/errata.py
@@ -0,0 +1,87 @@
+#!/usr/bin/python
+
+# Client code for Update Agent
+# Copyright (c) 1999-2002 Red Hat, Inc.  Distributed under GPL.
+#
+# Author: Adrian Likins  4:
+# Newer sats send down arch, filter using name+arch
+for p in packagelist:
+	if current_packages_with_arch.has_key(p[0]+p[4]):
+	u[p[0]+p[4]] = p
+	elif current_packages_with_arch.has_key(p[0]+"noarch"):
+	u[p[0]+p[4]] = p
+	elif p[4] == "noarch" and current_packages.has_key(p[0]):
+	u[p[0]] = p
+else:
+# 5.2 and older sats + hosted dont send arch
+for p in packagelist:

Re: [Spacewalk-devel] erratas and the client plugin package action

2011-10-17 Thread Ionuț Arțăriși 

On 10/17/2011 04:19 PM, Jan Pazdziora wrote:

On Mon, Oct 17, 2011 at 04:05:46PM +0200, Ionuț Arțăriși wrote:


The whole point of this patch for us was so that we could install
suse patches with the special zypper way. In order for that to
happen we need to first check if the server supports this new API
command, so we need a new capability.

So can you please apply this patch? (zypp-plugin-spacewalk currently
depends on this capability being provided to install patches
correctly).

Thank you,
Ionuț
> From 35e7d4257942ac91c21d7120ed5d874032629b72 Mon Sep 17 00:00:00 2001
From: Michael Calmer
Date: Wed, 6 Jul 2011 17:51:04 +0200
Subject: [PATCH] add server capability xmlrpc.errata.patch_names'

---
  backend/server/rhnCapability.py |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/backend/server/rhnCapability.py b/backend/server/rhnCapability.py
index 1259747..ad6c306 100644
--- a/backend/server/rhnCapability.py
+++ b/backend/server/rhnCapability.py
@@ -186,6 +186,7 @@ def _set_server_capabilities():
  'rhncfg.filetype.directory' : {'version' : 1, 'value' : 
1},
  'xmlrpc.packages.extended_profile'  : {'version' : '1-2', 'value' 
: 1},
  'xmlrpc.packages.suse_products' : {'version' : 1, 'value' : 
1},
+'xmlrpc.errata.patch_names' : {'version' : 1, 'value' : 1},

Nack -- it does not apply cleanly in Spacewalk master. We don't have
that xmlrpc.packages.suse_products at all so I'm affraid something is
getting lost in the translation.

Also, you might want to put the comment you have in the mail to the
commit message so that we have the documentation of this capability
preserved at least in the commit message.


Thanks, this should fix it.
>From cbda448d1f5834a8ebe39d1695ca6bce6cd6b1cc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ionu=C8=9B=20Ar=C8=9B=C4=83ri=C8=99i?= 
Date: Mon, 17 Oct 2011 16:43:55 +0200
Subject: [PATCH] add an 'xmlrpc.errata.patch_names' capability

This capability corresponds to the getErrataNamesById action in
xmlrpc.errata. zypp-plugin-spacewalk depends on it for installing SUSE
patches as patches instead of individual packages (i.e. zypper install
patch:PatchName1 patch:PatchName2)
---
 backend/server/rhnCapability.py |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/backend/server/rhnCapability.py b/backend/server/rhnCapability.py
index fef1f32..5ac0fc6 100644
--- a/backend/server/rhnCapability.py
+++ b/backend/server/rhnCapability.py
@@ -186,6 +186,7 @@ def _set_server_capabilities():
 'rhncfg.content.base64_decode'  : {'version' : 1, 'value' : 1},
 'rhncfg.filetype.directory' : {'version' : 1, 'value' : 1},
 'xmlrpc.packages.extended_profile'  : {'version' : '1-2', 'value' : 1},
+'xmlrpc.errata.patch_names' : {'version' : 1, 'value' : 1},
 'staging_content'   : {'version' : 1, 'value' : 1},
 'ipv6'  : {'version' : 1, 'value' : 1},
 }
-- 
1.7.3.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] erratas and the client plugin package action

2011-10-17 Thread Ionuț Arțăriși 

On 06/01/2011 10:31 AM, Jan Pazdziora wrote:

I looked a bit more into rhnSQL and I found two more helpers in
rhnSQL.sql_lib. It looks like a good place for adding the bind_list
function.

You patch is now in Spacewalk master,
49df1fa935453bbb3fa027d31859b44dfec6c32d. I just polished a couple of
trailing whitespaces.

Thank you!

The whole point of this patch for us was so that we could install suse 
patches with the special zypper way. In order for that to happen we need 
to first check if the server supports this new API command, so we need a 
new capability.


So can you please apply this patch? (zypp-plugin-spacewalk currently 
depends on this capability being provided to install patches correctly).


Thank you,
Ionuț
>From 35e7d4257942ac91c21d7120ed5d874032629b72 Mon Sep 17 00:00:00 2001
From: Michael Calmer 
Date: Wed, 6 Jul 2011 17:51:04 +0200
Subject: [PATCH] add server capability xmlrpc.errata.patch_names'

---
 backend/server/rhnCapability.py |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/backend/server/rhnCapability.py b/backend/server/rhnCapability.py
index 1259747..ad6c306 100644
--- a/backend/server/rhnCapability.py
+++ b/backend/server/rhnCapability.py
@@ -186,6 +186,7 @@ def _set_server_capabilities():
 'rhncfg.filetype.directory' : {'version' : 1, 'value' : 1},
 'xmlrpc.packages.extended_profile'  : {'version' : '1-2', 'value' : 1},
 'xmlrpc.packages.suse_products' : {'version' : 1, 'value' : 1},
+'xmlrpc.errata.patch_names' : {'version' : 1, 'value' : 1},
 'staging_content'   : {'version' : 1, 'value' : 1},
 }
 l = []
-- 
1.7.3.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] VARCHAR2 BYTEs and CHARs

2011-09-22 Thread Ionuț Arțăriși 

On 09/12/2011 11:00 AM, Jan Pazdziora wrote:

On Wed, Jul 27, 2011 at 03:47:39PM +0200, Ionuț Arțăriși wrote:

Adding a char to varchar2 definitions would then be step to, and piece
of cake.

Hm... writing another tool to do what chameleon currently does seems
like a massive undertaking.

Kudos to Michal Mráka who did just that.

More kudos from me as well!


The Spacewalk master/nightly build process is now chameleon-free, so
if you like, we can move forward with adding the CHAR declaration to
the schema definition -- I'll be happy to review a patch.

In the patch, you'll also need to amend spacewalk-oracle2postgresql to
remove the CHAR from the declaration as PostgreSQL does not support
it, and add schema upgrade scripts to modify existing installation.
I've attached the patches for the new regexp + migration and schema 
change the way I've applied them in our repository.

Well, before working on the patch, we should probably decide if the
changes should only touch columns where you've already hit the
problem, or if you want to change all VARCHAR2 columns in the database.
We have multiple columns like LABEL where we only allow limited subset
of US-ASCII (typically a-zA-Z0-9_ or something similar), so there it
might actually be usefull to keep it BYTE (and explicitly declare it
as BYTE) and perhaps also add a CHECK constraint which will enforce
this limited subset on the column.

I wasn't aware of this specific case and since I don't know how many 
other columns would be in this same situation I can't advocate changing 
to CHAR for all of the VARCHAR2 columns. However I do think that this 
specific constraint should only be a CHECK constraint and the storage 
format should be irrelevant.


-Ionuț
>From a8f017ed753394d9fbd4a9cb2069f6cbb1294e60 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ionu=C8=9B=20Ar=C8=9B=C4=83ri=C8=99i?= 
Date: Thu, 22 Sep 2011 13:28:32 +0200
Subject: [PATCH 1/2] allow setting VARCHAR2 size in CHARs and BYTEs
 explicitly

---
 schema/spacewalk/spacewalk-oracle2postgresql |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/schema/spacewalk/spacewalk-oracle2postgresql b/schema/spacewalk/spacewalk-oracle2postgresql
index a11d6f4..eb0193e 100755
--- a/schema/spacewalk/spacewalk-oracle2postgresql
+++ b/schema/spacewalk/spacewalk-oracle2postgresql
@@ -1,10 +1,11 @@
 #!/bin/sed -f
 
 # data types
-s/\bVARCHAR2/VARCHAR/i;
 s/\bDATE\b/TIMESTAMPTZ/i;
 s/\bNUMBER\b/NUMERIC/i;
 s/\bBLOB\b/BYTEA/i;
+# postgres doesn't have CHAR or BYTE
+s/\bVARCHAR2(\([0-9]\+\)[[:blank:]]*\(CHAR\|BYTE\)\?)/VARCHAR(\1)/i;
 
 # functions
 s/\bSYSDATE\b/CURRENT_TIMESTAMP/i;
-- 
1.7.6.1

>From 1d3466464e59da565f562816493764987532d521 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ionu=C8=9B=20Ar=C8=9B=C4=83ri=C8=99i?= 
Date: Thu, 22 Sep 2011 13:28:59 +0200
Subject: [PATCH 2/2] change rhnServerAction.result_msg column size from
 default BYTEs to CHARs

---
 schema/spacewalk/common/tables/rhnServerAction.sql |2 +-
 .../001-rhnServerAction-result_msg-size.sql|3 +++
 2 files changed, 4 insertions(+), 1 deletions(-)
 create mode 100644 schema/spacewalk/upgrade/spacewalk-schema-1.6-to-spacewalk-schema-1.7/001-rhnServerAction-result_msg-size.sql

diff --git a/schema/spacewalk/common/tables/rhnServerAction.sql b/schema/spacewalk/common/tables/rhnServerAction.sql
index 292c83e..8b15128 100644
--- a/schema/spacewalk/common/tables/rhnServerAction.sql
+++ b/schema/spacewalk/common/tables/rhnServerAction.sql
@@ -27,7 +27,7 @@ CREATE TABLE rhnServerAction
  CONSTRAINT rhn_server_action_status_fk
  REFERENCES rhnActionStatus (id),
 result_code  NUMBER,
-result_msg   VARCHAR2(1024),
+result_msg   VARCHAR2(1024 CHAR),
 pickup_time  DATE,
 remaining_tries  NUMBER
  DEFAULT (5) NOT NULL,
diff --git a/schema/spacewalk/upgrade/spacewalk-schema-1.6-to-spacewalk-schema-1.7/001-rhnServerAction-result_msg-size.sql b/schema/spacewalk/upgrade/spacewalk-schema-1.6-to-spacewalk-schema-1.7/001-rhnServerAction-result_msg-size.sql
new file mode 100644
index 000..7b41ff3
--- /dev/null
+++ b/schema/spacewalk/upgrade/spacewalk-schema-1.6-to-spacewalk-schema-1.7/001-rhnServerAction-result_msg-size.sql
@@ -0,0 +1,3 @@
+alter table rhnServerAction modify ( result_msg VARCHAR2(1024 CHAR) );
+
+commit;
-- 
1.7.6.1

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

[Spacewalk-devel] Duplicate files in perl-NOCPulse-Probe

2011-09-08 Thread Ionuț Arțăriși 

Hi,

I found more duplicate files. SQLPlus.pm is present in both the main 
package and the -Oracle subpackage of perl-NOCPulse-Probe.


Thanks,
Ionuț
>From fa40ee65b2df036dc28566743bc0d2fcc9430049 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ionu=C8=9B=20Ar=C8=9B=C4=83ri=C8=99i?= 
Date: Thu, 8 Sep 2011 14:06:46 +0200
Subject: [PATCH] remove duplicated SQLPlus.pm file from main package as it is
 already in -Oracle

---
 .../PerlModules/NP/Probe/perl-NOCpulse-Probe.spec  |   12 +++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/monitoring/PerlModules/NP/Probe/perl-NOCpulse-Probe.spec b/monitoring/PerlModules/NP/Probe/perl-NOCpulse-Probe.spec
index 0fe4a42..6c53ef4 100644
--- a/monitoring/PerlModules/NP/Probe/perl-NOCpulse-Probe.spec
+++ b/monitoring/PerlModules/NP/Probe/perl-NOCpulse-Probe.spec
@@ -82,6 +82,7 @@ rm -rf $RPM_BUILD_ROOT
 %dir %{perl_vendorlib}/NOCpulse
 %dir %{perl_vendorlib}/NOCpulse/Probe
 %dir %{perl_vendorlib}/NOCpulse/Probe/DataSource
+%dir %{perl_vendorlib}/NOCpulse/Probe/Shell
 %{perl_vendorlib}/NOCpulse/Probe/Config*
 %{perl_vendorlib}/NOCpulse/Probe/DataSource/AbstractDataSource.pm
 %{perl_vendorlib}/NOCpulse/Probe/DataSource/AbstractDatabase.pm
@@ -124,7 +125,16 @@ rm -rf $RPM_BUILD_ROOT
 %{perl_vendorlib}/NOCpulse/Probe/DataSource/test/TestWindowsCommand.pm
 %{perl_vendorlib}/NOCpulse/Probe/*.pm
 %{perl_vendorlib}/NOCpulse/Probe/SNMP*
-%{perl_vendorlib}/NOCpulse/Probe/Shell*
+%{perl_vendorlib}/NOCpulse/Probe/Shell/AbstractShell.pm
+%{perl_vendorlib}/NOCpulse/Probe/Shell/CannedWindowsService.pm
+%{perl_vendorlib}/NOCpulse/Probe/Shell/Local.pm
+%{perl_vendorlib}/NOCpulse/Probe/Shell/SSH.pm
+%{perl_vendorlib}/NOCpulse/Probe/Shell/Unix.pm
+%{perl_vendorlib}/NOCpulse/Probe/Shell/WindowsService.pm
+%{perl_vendorlib}/NOCpulse/Probe/Shell/test
+%{perl_vendorlib}/NOCpulse/Probe/Shell/test/TestSQLPlus.pm
+%{perl_vendorlib}/NOCpulse/Probe/Shell/test/TestShell.pm
+%{perl_vendorlib}/NOCpulse/Probe/Shell/test/TestWindowsService.pm
 %{perl_vendorlib}/NOCpulse/Probe/Utils*
 %{perl_vendorlib}/NOCpulse/Probe/test*
 %{_mandir}/man3/NOCpulse::Probe::DataSource*
-- 
1.7.4.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] Audit Logging

2011-08-29 Thread Ionuț Arțăriși 

On 08/29/2011 03:14 PM, Cliff Perry wrote:

One of the main design decisions I would consider for behaviour is...

If the logging daemon is down, do all actions fail - or do you still
allow successful actions, knowing that it cannot log the event.
  - Or do you have a fall-back temporary on-disk cache to be played back
into the logger once it is running again.

In short, my personal preference is not to become a toaster if audit
logging has crashed (including out of space to write).

Cliff

We had a discussion about this as well and we decided that if it can't 
be logged then it shouldn't happen at all. We want the admin to always 
know what modifications have been done on the client systems. It is not 
acceptable for some actions to complete and the admin to not be able to 
see them in the logs (or see them later - there is no way to guarantee 
that the on-disk cache will be played back into the logger, but the 
action will still have been completed).


So if the auditlogging server is down, then the XMLRPC API is down. The 
whole point of audit logging is to not let anything "important" pass 
through without having a record of it. You can of course turn the 
logging off from the server.


-Ionuț

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] Audit Logging

2011-08-29 Thread Ionuț Arțăriși 
Just to add to the discussion, I'll tell you how I thought about logging 
the python backend XMLRPC API calls. This needs a lot less filtering 
than logging the WebUI though, since we're interested in most of the 
methods, so it's a lot easier.


We decided we only wanted to log successful API calls that are deemed 
"important". So far this means that the api method requires 
authentication (and therefore the systemid XML) file to be sent with it.


In the apacheRequest, after the function is called and before the 
response is sent back, an auditlog function is called with all the 
information about the XMLRPC method that was called, its arguments and 
the request. I think this is the only central place from which the 
logging function can be called. The auditlog function belongs to a 
separate module and it does all the processing to extract the 
information that we deem useful (ips/hosts/serverId etc.). It also 
decides if the call is important enough to be logged or just doesn't do 
anything if auditlogging is turned off in an rhn.conf file. It calls the 
auditlogging XMLRPC server with the result and also crashes the whole 
API request if anything goes wrong with the logging.


Feedback appreciated :)

-Ionuț

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

[Spacewalk-devel] duplicated files in SputLite-server and SputLite-client packages

2011-08-16 Thread Ionuț Arțăriși 

Hello,

I'm trying to reduce the amount of packages that own the same set of files.

SputLite-server and SputLite-client both explicitly own 
/usr/lib/perl5/vendor_perl/5.10.0/NOCpulse/CommandQueue*


Is this intentional or is it just a packaging bug? If it's a bug, then 
this patch would help.


Thanks,
-Ionuț
>From a998cf44e01e63a665679a2936e8804bb161abb1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ionu=C8=9B=20Ar=C8=9B=C4=83ri=C8=99i?= 
Date: Tue, 16 Aug 2011 11:07:10 +0200
Subject: [PATCH] remove perl module files from the -server package

---
 monitoring/SputLite/SputLite.spec |2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/monitoring/SputLite/SputLite.spec b/monitoring/SputLite/SputLite.spec
index c788138..4d287e5 100644
--- a/monitoring/SputLite/SputLite.spec
+++ b/monitoring/SputLite/SputLite.spec
@@ -83,7 +83,6 @@ fi
 %files server
 %defattr(-,root,root,-)
 %attr(755, nocpulse, nocpulse) %dir %templatedir
-%{perl_vendorlib}/NOCpulse/*
 %cgi_bin/*
 %cgi_mod_perl/*
 %templatedir/*
@@ -93,7 +92,6 @@ fi
 %attr(755,nocpulse,nocpulse) %dir %{vardir}/commands
 %attr(755,nocpulse,nocpulse) %dir %{vardir}/queue/commands
 %{_bindir}/*
-%dir %{perl_vendorlib}/NOCpulse/CommandQueue
 %{perl_vendorlib}/NOCpulse/*
 
 %clean
-- 
1.7.4.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] VARCHAR2 BYTEs and CHARs

2011-07-27 Thread Ionuț Arțăriși 

On 07/22/2011 10:48 AM, Jan Pazdziora wrote:

On Wed, Jul 20, 2011 at 04:39:14PM +0200, Ionuț Arțăriși wrote:

Hello,

We've recently come upon a bug when trying to insert a string of 1024
unicode characters into a database column of VARCHAR2(1024 BYTE). Rather
than dealing with string encoding transformations everywhere this
issue pops up
in the code, I thought that modifying the database schema would be the
better solution.

BYTE seems to be the default for the Oracle database, but unicode
strings would map better to VARCHAR2(1024 CHAR).

The problem is that I couldn't find any way to set this in the table
schema files and get it past chameleon. chameleon just doesn't have the
right grammar defined to parse VARCHAR2(1024 CHAR) or "alter session set
nls_length_semantics=char".

So I was wondering if you guys know more about this problem or a better
solution than patching chameleon.

Replacing chameleon with a custom script (sed? perl?) which would do
the equivalent transformation but be easier to be extended would be my
preferred option.

I'll be happy to review a patch which would do that.

Adding a char to varchar2 definitions would then be step to, and piece
of cake.

Hm... writing another tool to do what chameleon currently does seems 
like a massive undertaking.


Besides, the chameleon code looks quite clean and easy enough to extend. 
Here's a patch that allows specifying the VARCHAR2 data type (for lack 
of a better name).


Another question would be: do you guys have commit access to the 
upstream chameleon project?


-Ionuț
Index: parser.py
===
--- parser.py   (revision 8)
+++ parser.py   (working copy)
@@ -40,6 +40,7 @@
 'UNIQUE',
 'CONSTRAINT',
 'REFERENCES',
+'BYTE',
 'BYTEA',
 'BLOB',
 'CHAR',
@@ -461,7 +462,14 @@
 p[0] = ','.join((p[1], p[3]))
 else:
 p[0] = p[1]
-
+
+def p_datatype(p):
+"""
+datatype : CHAR
+| BYTE
+"""
+p[0] = p[1]
+
 def p_type(p):
 """
 type : CHAR
@@ -470,6 +478,7 @@
 | CHAR LPAREN DIGITS RPAREN
 | VARCHAR LPAREN DIGITS RPAREN
 | VARCHAR2 LPAREN DIGITS RPAREN
+| VARCHAR2 LPAREN DIGITS datatype RPAREN
 | NUMBER
 | NUMBER LPAREN precision RPAREN
 | NUMERIC
@@ -482,8 +491,10 @@
 | EVR_T
 """
 dt = Type(p[1])
-if len(p) == 5:
+if len(p) >= 5:
 dt.precision = p[3]
+if len(p) == 6:
+dt.datatype = p[4]
 p[0] = dt
 
 def p_notnull(p):
Index: model.py
===
--- model.py(revision 8)
+++ model.py(working copy)
@@ -150,9 +150,10 @@
 
 class Type(Object):
 
-def __init__(self, name, precision=None):
+def __init__(self, name, precision=None, datatype=None):
 Object.__init__(self, name)
 self.precision = precision
+self.datatype = datatype
 
 
 class Index(Object):
@@ -349,4 +350,4 @@
 self.terms = terms
 
 class Include(Object):
-pass
\ No newline at end of file
+pass
Index: oracle.py
===
--- oracle.py   (revision 8)
+++ oracle.py   (working copy)
@@ -92,7 +92,7 @@
 'SMALLINT': lambda t : model.Type('NUMBER', 5),
 'INTEGER' : lambda t : model.Type('NUMBER', 10),
 'BIGINT'  : lambda t : model.Type('NUMBER', 19),
-'VARCHAR' : lambda t : model.Type('VARCHAR2', t.precision),
+'VARCHAR' : lambda t : model.Type('VARCHAR2', t.precision, t.datatype),
 'BYTEA'   : lambda t : model.Type('BLOB'),
 }
 
@@ -107,9 +107,10 @@
 t = self.xlated(type)
 s = []
 s.append(t.name.upper())
-if t.precision is not None:
-s.append('(')
-s.append(str(t.precision))
+if t.precision:
+s.append('(' + str(t.precision))
+if t.datatype:
+s.append(' ' + str(t.datatype))
 s.append(')')
 return self.join(s)
 
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

[Spacewalk-devel] VARCHAR2 BYTEs and CHARs

2011-07-20 Thread Ionuț Arțăriși 

Hello,

We've recently come upon a bug when trying to insert a string of 1024
unicode characters into a database column of VARCHAR2(1024 BYTE). Rather
than dealing with string encoding transformations everywhere this issue 
pops up

in the code, I thought that modifying the database schema would be the
better solution.

BYTE seems to be the default for the Oracle database, but unicode
strings would map better to VARCHAR2(1024 CHAR).

The problem is that I couldn't find any way to set this in the table
schema files and get it past chameleon. chameleon just doesn't have the
right grammar defined to parse VARCHAR2(1024 CHAR) or "alter session set
nls_length_semantics=char".

So I was wondering if you guys know more about this problem or a better
solution than patching chameleon. Is there a specific reason for not 
using VARCHAR2( CHAR)?


Thanks,
Ionuț

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] erratas and the client plugin package action

2011-05-24 Thread Ionuț Arțăriși 

On 05/23/2011 04:45 PM, Jan Pazdziora wrote:

On Thu, May 19, 2011 at 11:46:37AM +0200, Ionuț Arțăriși wrote:

On 05/18/2011 05:05 PM, Jan Pazdziora wrote:

On Wed, May 18, 2011 at 02:38:54PM +0200, Ionuț Arțăriși wrote:

On 05/18/2011 01:14 PM, Jan Pazdziora wrote:

...

Nack. This is SQL-injection-prone. You have to use bind parameters
or sanitize the input properly.

Thanks, I have fixed the SQL issue.

It's still somewhat missing in your patch.

Ok, I think I now understood what you mean. Here's the re-patched patch :).

Good.

Now all that is left is make sure the call cannot be used to access
information which should not be accessible to the server. If you check
the getErrataInfo and take it as an example, you will see how to
authenticate / authorize, and we will need the query extended to join
with (probably) rhnServerChannel and rhnChannelErrata.



Thanks, I think this should be fixed now.

Those SQL IN operations seem to be quite tedious. Is there anywhere
that we could move this _bind_list function? Perhaps to something
like rhnSQL.bind_list? I haven't found any other helpers like this
already in rhnSQL, but I've seen it used in other places.

It is certainly possible.

I looked a bit more into rhnSQL and I found two more helpers in 
rhnSQL.sql_lib. It looks like a good place for adding the bind_list 
function.


-Ionuț
>From 3688631b956423aa3a0b31d370f98b177462272a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ionu=C8=9B=20Ar=C8=9B=C4=83ri=C8=99i?= 
Date: Tue, 24 May 2011 14:53:40 +0200
Subject: [PATCH] added errata.getErrataNamesById function to the API

---
 backend/server/handlers/xmlrpc/errata.py |   56 ++
 1 files changed, 56 insertions(+), 0 deletions(-)

diff --git a/backend/server/handlers/xmlrpc/errata.py b/backend/server/handlers/xmlrpc/errata.py
index 5b11637..ef161cd 100644
--- a/backend/server/handlers/xmlrpc/errata.py
+++ b/backend/server/handlers/xmlrpc/errata.py
@@ -35,6 +35,7 @@ class Errata(rhnHandler):
 self.functions.append('GetByPackage')  # Clients v1-
 self.functions.append('getPackageErratum') # Clients v2+
 self.functions.append('getErrataInfo') # clients v2+
+self.functions.append('getErrataNamesById')
 
 def GetByPackage(self, pkg, osRel):
 """ Clients v1- Get errata for a package given "n-v-r" format
@@ -242,7 +243,62 @@ class Errata(rhnHandler):
 pkg_arch])
 return ret
 
+def getErrataNamesById(self, system_id, errata_ids):
+"""Return a list of RhnErrata tuples of (id, advisory_name)
 
+IN: system_id - id of the system requesting this info (must be
+subscribed to the channel that contains the erratas)
+errata_ids - a list of RhnErrata ids
+
+Only the erratas that belong to channels that the client system
+is subscribed to are returned. If no erratas match this
+criterion, then an empty list is returned.
+
+"""
+log_debug(5, system_id, errata_ids)
+self.auth_system(system_id)
+
+log_debug(1, self.server_id, errata_ids)
+
+sql_list, bound_vars = _bind_list(errata_ids)
+bound_vars.update({'server_id': self.server_id})
+
+sql = """SELECT DISTINCT e.id, e.advisory_name
+ FROM rhnErrata e,
+  rhnPackage p,
+  rhnChannelPackage cp, 
+  rhnServerChannel sc,
+  rhnErrataPackage ep
+ WHERE e.id in (%s) AND
+   ep.errata_id = e.id AND
+   ep.package_id = p.id AND
+   sc.server_id = :server_id AND
+   sc.channel_id = cp.channel_id AND
+   cp.package_id = p.id"""
+h = rhnSQL.prepare(sql % sql_list)
+h.execute(**bound_vars)
+
+return h.fetchall()
+
+
+def _bind_list(elems):
+"""Transform a list into an sql list with bound parameters
+
+IN: elems - a list of elements
+
+Returns a tuple of:
+ sql_list - a comma separated list of parameter numbers: 'p_0, p_1, p_2'
+ bound_vars - a dict of parameter names and values {'p_0': 42, 'p_1': 34}
+
+"""
+bound_names = []
+bound_vars = {}
+for i, elem in enumerate(elems):
+bound_vars['p_%s' % i] = elem
+bound_names.append(':p_%s' % i)
+sql_list = ', '.join(bound_names)
+return sql_list, bound_vars
+
 #-
 if __name__ == "__main__":
 print "You can not run this module by itself"
-- 
1.7.4.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] erratas and the client plugin package action

2011-05-19 Thread Ionuț Arțăriși 

On 05/18/2011 05:05 PM, Jan Pazdziora wrote:

On Wed, May 18, 2011 at 02:38:54PM +0200, Ionuț Arțăriși wrote:

On 05/18/2011 01:14 PM, Jan Pazdziora wrote:

...

Nack. This is SQL-injection-prone. You have to use bind parameters
or sanitize the input properly.

Thanks, I have fixed the SQL issue.

It's still somewhat missing in your patch.


Ok, I think I now understood what you mean. Here's the re-patched patch :).

Those SQL IN operations seem to be quite tedious. Is there anywhere that 
we could move this _bind_list function? Perhaps to something like 
rhnSQL.bind_list? I haven't found any other helpers like this already in 
rhnSQL, but I've seen it used in other places.


-Ionuț
>From a519ba5cef71bdec3bf6fa2e42438b00c34af14c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ionu=C8=9B=20Ar=C8=9B=C4=83ri=C8=99i?= 
Date: Wed, 18 May 2011 12:31:59 +0200
Subject: [PATCH] added errata.getErrataNamesById function to the API

---
 backend/server/handlers/xmlrpc/errata.py |   36 ++
 1 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/backend/server/handlers/xmlrpc/errata.py b/backend/server/handlers/xmlrpc/errata.py
index 5b11637..cbaab6e 100644
--- a/backend/server/handlers/xmlrpc/errata.py
+++ b/backend/server/handlers/xmlrpc/errata.py
@@ -35,6 +35,7 @@ class Errata(rhnHandler):
 self.functions.append('GetByPackage')  # Clients v1-
 self.functions.append('getPackageErratum') # Clients v2+
 self.functions.append('getErrataInfo') # clients v2+
+self.functions.append('getErrataNamesById')
 
 def GetByPackage(self, pkg, osRel):
 """ Clients v1- Get errata for a package given "n-v-r" format
@@ -242,7 +243,42 @@ class Errata(rhnHandler):
 pkg_arch])
 return ret
 
+def getErrataNamesById(self, errata_ids):
+"""Return a list of RhnErrata tuples of (id, advisory_name)
 
+IN: errata_ids - a list of RhnErrata ids
+
+Returns an empty list if no erratas were found for the provided ids.
+
+"""
+sql_list, bound_vars = _bind_list(errata_ids)
+
+sql = """SELECT id, advisory_name FROM RhnErrata
+ WHERE id IN (%s)"""
+h = rhnSQL.prepare(sql % sql_list)
+h.execute(**bound_vars)
+
+return h.fetchall()
+
+
+def _bind_list(elems):
+"""Transform a list into an sql list with bound parameters
+
+IN: elems - a list of elements
+
+Returns a tuple of:
+ sql_list - a comma separated list of parameter numbers: 'p_0, p_1, p_2'
+ bound_vars - a dict of parameter names and values {'p_0': 42, 'p_1': 34}
+
+"""
+bound_names = []
+bound_vars = {}
+for i, elem in enumerate(elems):
+bound_vars['p_%s' % i] = elem
+bound_names.append(':p_%s' % i)
+sql_list = ', '.join(bound_names)
+return sql_list, bound_vars
+
 #-
 if __name__ == "__main__":
 print "You can not run this module by itself"
-- 
1.7.3.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] erratas and the client plugin package action

2011-05-18 Thread Ionuț Arțăriși 

On 05/18/2011 01:14 PM, Jan Pazdziora wrote:

...

Nack. This is SQL-injection-prone. You have to use bind parameters
or sanitize the input properly.

Thanks, I have fixed the SQL issue.


Besides, if you allow the list of errata id's to be passed in, which
would lead to multiple erratas to be returned, shouldn't you return
the id as well to make it clear which advisory name belongs to which
id?


We don't exactly need the errata ids, but I can see how this might be 
useful, so I have changed the method to return a list of (id, 
advisory_name) tuples.


-Ionuț
>From 2294cbcf78713d600f716aa202a812df7d6480be Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ionu=C8=9B=20Ar=C8=9B=C4=83ri=C8=99i?= 
Date: Wed, 18 May 2011 12:31:59 +0200
Subject: [PATCH] added errata.getErrataNamesById function to the API

---
 backend/server/handlers/xmlrpc/errata.py |   20 
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/backend/server/handlers/xmlrpc/errata.py b/backend/server/handlers/xmlrpc/errata.py
index 5b11637..4f1abcd 100644
--- a/backend/server/handlers/xmlrpc/errata.py
+++ b/backend/server/handlers/xmlrpc/errata.py
@@ -35,6 +35,7 @@ class Errata(rhnHandler):
 self.functions.append('GetByPackage')  # Clients v1-
 self.functions.append('getPackageErratum') # Clients v2+
 self.functions.append('getErrataInfo') # clients v2+
+self.functions.append('getErrataNamesById')
 
 def GetByPackage(self, pkg, osRel):
 """ Clients v1- Get errata for a package given "n-v-r" format
@@ -243,6 +244,25 @@ class Errata(rhnHandler):
 return ret
 
 
+def getErrataNamesById(self, errata_ids):
+"""Return a list of RhnErrata tuples of (id, advisory_name)
+
+:arg errata_ids: a list of RhnErrata ids
+
+Returns an empty list if no erratas were found for the provided ids.
+
+"""
+# transform the list of ints to an sql list that we can forcibly
+# insert into the sql statement
+sql_list = ', '.join([str(i) for i in errata_ids])
+
+sql = """SELECT id, advisory_name FROM RhnErrata
+ WHERE id IN (%s)"""
+h = rhnSQL.prepare(sql % sql_list)
+h.execute()
+
+return h.fetchall()
+
 #-
 if __name__ == "__main__":
 print "You can not run this module by itself"
-- 
1.7.3.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] erratas and the client plugin package action

2011-05-18 Thread Ionuț Arțăriși 


Thanks a lot for your comprehensive answer.

On 05/17/2011 04:06 PM, Miroslav Suchý wrote:

errata.update only receives a list of errata ids, how could we get the
names from the server in order to send them to zypper? I looked around

Hmm, I thought that zypper code could handle it already. How did you
done it till now?

Right now it's done the same way that yum does it - it receives the list 
of packages and installs them as normal packages. This approach doesn't 
work so well for zypper though.



Could we add a getErrataNamesById method the xmlrpc API under errata?

Yes, you can.

Generally - if you need some API (backend or frontend) and you did not
change semantics of old ones. And as far as the new API call does not
create security risk (e.g. providing info, which system should not see).
Then you can add it without problem.

I have attached a patch to add a new method to the API which returns the 
names of erratas when given a list of errata ids. We need this so we can 
call it from actions/errata.py in the zypp-plugin-spacewalk.


-Ionuț


>From 4c72b6bf32872377e427bcc53cfefe8e59e69895 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ionu=C8=9B=20Ar=C8=9B=C4=83ri=C8=99i?= 
Date: Wed, 18 May 2011 12:31:59 +0200
Subject: [PATCH] added errata.getErrataNamesById function to the API

---
 backend/server/handlers/xmlrpc/errata.py |   21 +
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/backend/server/handlers/xmlrpc/errata.py b/backend/server/handlers/xmlrpc/errata.py
index 5b11637..b5fbada 100644
--- a/backend/server/handlers/xmlrpc/errata.py
+++ b/backend/server/handlers/xmlrpc/errata.py
@@ -35,6 +35,7 @@ class Errata(rhnHandler):
 self.functions.append('GetByPackage')  # Clients v1-
 self.functions.append('getPackageErratum') # Clients v2+
 self.functions.append('getErrataInfo') # clients v2+
+self.functions.append('getErrataNamesById')
 
 def GetByPackage(self, pkg, osRel):
 """ Clients v1- Get errata for a package given "n-v-r" format
@@ -243,6 +244,26 @@ class Errata(rhnHandler):
 return ret
 
 
+def getErrataNamesById(self, errata_ids):
+"""Return a list of RhnErrata names
+
+:arg errata_ids: a list of RhnErrata ids
+
+Returns an empty list if no erratas were found for the provided ids.
+
+"""
+# transform the list of ints to an sql list that we can forcibly
+# insert into the sql statement
+sql_list = ', '.join([str(i) for i in errata_ids])
+
+sql = """SELECT advisory_name FROM rhnerrata
+ WHERE id IN (%s)""" % sql_list
+h = rhnSQL.prepare(sql)
+h.execute()
+erratas = [tup[0] for tup in h.fetchall()]
+
+return erratas
+
 #-
 if __name__ == "__main__":
 print "You can not run this module by itself"
-- 
1.7.3.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] erratas and the client plugin package action

2011-05-17 Thread Ionuț Arțăriși 

On 05/12/2011 03:26 PM, Duncan Mac-Vicar P. wrote:
I would like to have the name of the errata so that the client solver 
can figure the right packages to install (we have the errata in the 
repo metadata as well). But right now it looks like the package list 
is sent down.


Is there any way to get the errata names at all right now? Since 
errata.update only receives a list of errata ids, how could we get the 
names from the server in order to send them to zypper? I looked around 
the XMLRPC API a bit and besides being deceived by the name of the 
getErrataInfo method(which only returns a list of packages - can we 
rename it to getPackageList?), I couldn't find a way to do this.


Could we add a getErrataNamesById method the xmlrpc API under errata?

-Ionuț

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] Misleading parameter usage help for rhncfg-manager diff

2011-05-10 Thread Ionuț Arțăriși 
It seems that this thread was buried. I'll try to resurrect it with some 
patches.


0001 - fix usage documentation messages for topdir and dest-file
0002 - remove unused import, fix indentation and a minor typo

-Ionuț
>From 20e888f8091e4c659b8e96a15c79fdc15c18cb97 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ionu=C8=9B=20Ar=C8=9B=C4=83ri=C8=99i?= 
Date: Tue, 10 May 2011 14:55:51 +0200
Subject: [PATCH 1/2] fix usage documentation messages for topdir and
 dest-file

---
 .../tools/rhncfg/config_management/rhncfg_diff.py  |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/client/tools/rhncfg/config_management/rhncfg_diff.py b/client/tools/rhncfg/config_management/rhncfg_diff.py
index d497ff0..759279c 100644
--- a/client/tools/rhncfg/config_management/rhncfg_diff.py
+++ b/client/tools/rhncfg/config_management/rhncfg_diff.py
@@ -38,11 +38,11 @@ class Handler(handler_base.HandlerBase):
  ),
 handler_base.HandlerBase._option_class(
 '-d', '--dest-file',action="store",
- help="Upload the file as this path",
+ help="Remote file to compare to",
  ),
 handler_base.HandlerBase._option_class(
 '-t', '--topdir',   action="store",
- help="Make all files relative to this string",
+ help="Directory to which all file paths are relative",
  ),
 ]
 def run(self):
-- 
1.7.4.4

>From a62af83de44214071606ff98cb3a6773f92e9d6e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ionu=C8=9B=20Ar=C8=9B=C4=83ri=C8=99i?= 
Date: Tue, 10 May 2011 14:56:39 +0200
Subject: [PATCH 2/2] remove unused import, fix indentation and a minor typo

---
 .../tools/rhncfg/config_management/rhncfg_diff.py  |9 -
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/client/tools/rhncfg/config_management/rhncfg_diff.py b/client/tools/rhncfg/config_management/rhncfg_diff.py
index 759279c..b260dc1 100644
--- a/client/tools/rhncfg/config_management/rhncfg_diff.py
+++ b/client/tools/rhncfg/config_management/rhncfg_diff.py
@@ -21,7 +21,6 @@ import sys
 
 from config_common import handler_base, utils, cfg_exceptions
 from config_common.rhn_log import log_debug, die
-from config_common.file_utils import diff
 from spacewalk.common.fileutils import f_date, ostr_to_sym
 
 
@@ -121,14 +120,14 @@ class Handler(handler_base.HandlerBase):
 if info['encoding'] == 'base64':
 info['file_contents'] = base64.decodestring(info['file_contents'])
 else:
-die(9, 'Error: unknow encoding %s' % info['encoding'])
+die(9, 'Error: unknown encoding %s' % info['encoding'])
 except cfg_exceptions.RepositoryFileMissingError:
 die(2, "Error: no such file %s (revision %s) in config channel %s"
 % (path, revision, channel))
 if os.path.islink(local_file) and info['filetype'] != 'symlink' :
- die(8, "Cannot diff %s; the file on the system is a symbolic link while the file in the channel is not. " % local_file)
+die(8, "Cannot diff %s; the file on the system is a symbolic link while the file in the channel is not. " % local_file)
 if  info['filetype'] == 'symlink' and not os.path.islink(local_file) :
- die(8, "Cannot diff %s; the file on the system is not a symbolic link while the file in the channel is. " % local_file) 
+die(8, "Cannot diff %s; the file on the system is not a symbolic link while the file in the channel is. " % local_file) 
 if info['filetype'] == 'symlink':
 src_link = info['symlink']
 dest_link = os.readlink(local_file)
@@ -152,7 +151,7 @@ class Handler(handler_base.HandlerBase):
 if 'selinux_ctx' not in info:
 info['selinux_ctx'] = ''
 if not first_row and not self.__attributes_differ(info, local_info):
- return ""
+return ""
 else:
 template = "--- %s\t%s\tattributes: %s %s %s %s\tconfig channel: %s\trevision: %s"
 if not info.has_key('modified'):
-- 
1.7.4.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

[Spacewalk-devel] Misleading parameter usage help for rhncfg-manager diff

2011-04-14 Thread Ionuț Arțăriși 

Hello,

A documentation review found an issue with the usage information for the 
'rhncfg-manager diff' command. There are two parameters there with 
strange help text.


--dest-file help="Upload the file as this path" . This seems wrong 
because AFAICS there is no uploading involved with this command, it only 
diffs files. Besides changing the help text, maybe changing the 
parameter name to something more meaningful would be good (--remote-file 
comes to mind, but -r is already used for another option).
OTOH, isn't this parameter altogether superfluous as the same 
functionality is already provided by simply listing multiple files as 
arguments ("file [ file ... ]")? Is there a policy of having this 
command's parameters the same as another?


--topdir help="Make all files relative to this string". Here the help 
text is misleading because there are no new files being made, they are 
just being compared.


It looks like those two parameters were originally copied from 
rhncfg_add.py where they are exactly the same, only they make sense for 
that command.


-Ionuț

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel