Hi Milan
Many thanks for the pointers and review. I've tested the below on
both RHEL5 and RHEL6.
I've attached two patches, one for rhncfg-client list and one for
elist so they behave in the same manner.
Hope these are a bit better.
Thanks
CC
On Tue, May 27, 2014 at 5:41 PM, Milan Zázrivec <mzazri...@redhat.com> wrote:
> On Thursday 22 May 2014 09:20:26 Colin Coe wrote:
>> Hi all
>>
>> Please review this patch.
>>
>> It allows users of rhncfg-client elist to specify a list of files. For
>> example: rhncfg-client elist /etc/auto.hp /etc/auto.home /etc/auto.dummy
>> Using server name spacewalk.server.com
>> Argument List: 5
>> Mode Owner Group Size Rev Config Channel File
>> -rw-r--r-- root root 110 2 rhel6
>> /etc/auto.home
>> -rw-r--r-- root root 122 3 rhel6
>> /etc/auto.hp
>>
>> If a specified file does not exist it is ignored.
>>
>> This patch is useful because rhncfg-client elist can take a while to
>> return (depending on the number of managed files) and the user may
>> only be interested in one or two files.
>
> I think that this part:
>
> class Handler(handler_base.HandlerBase):
> def run(self):
> + print 'Argument List:', len(sys.argv)
> log_debug(2)
> r = self.repository
> files = r.list_files()
>
> is not very useful. It just prints number of command line arguments, which
> doesn't say much I'd say.
>
> In the next part of the patch:
>
> + if len(sys.argv) > 2:
> + arg_files = sys.argv[2:len(sys.argv)]
> +
> for file in files:
>
> + found = 0
> + if len(arg_files):
> + if not any(file[1] in f for f in arg_files):
> + continue
> +
> # Get the file info
> finfo = r.get_file_info(file[1])[1]
> # Get the file length
>
> I see the following problems:
>
> 1. we declare arg_files only if len(sys.argv) > 2 but later in the code we
> want to access arg_files (not matter what).
>
> 2. found = 0 is an unused variable
>
> 3. any() won't work on RHEL-5
>
> 4. any() is in fact not needed at all, because the whole if statement should
> just be:
>
> if len(arg_files) and not file[1] in arg_files:
> continue
>
> Also, this change should -- I think -- be reflected in the rhncfg-client
> manual page.
>
> I'm terribly sorry, but I have to say no to this patch. Otherwise this is
> a great idea and an useful extension, which we can certainly accept
> into our code, should the above points be addressed.
>
> Thank you Colin ;-)
> -MZ
>
> _______________________________________________
> Spacewalk-devel mailing list
> Spacewalk-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/spacewalk-devel
--
RHCE#805007969328369
--- /usr/share/rhn/config_client/rhncfgcli_elist.py.orig 2014-05-27
18:08:30.947676423 +0800
+++ /usr/share/rhn/config_client/rhncfgcli_elist.py 2014-05-27
18:13:58.185753228 +0800
@@ -1,6 +1,7 @@
from config_common.rhn_log import log_debug, die
from config_common.file_utils import ostr_to_sym
import handler_base, base64
+import sys
class Handler(handler_base.HandlerBase):
def run(self):
@@ -16,8 +17,15 @@
maxlen = max(maxlen, len(label)) + 2
print "%-10s %8s %-8s %10s %+3s %*s %s" % ('Mode', 'Owner',
'Group', 'Size', 'Rev', maxlen, label, "File")
+ arg_files = []
+ if len(sys.argv) > 2:
+ arg_files = sys.argv[2:len(sys.argv)]
+
for file in files:
+ if len(arg_files) and not file[1] in arg_files:
+ continue
+
# Get the file info
finfo = r.get_file_info(file[1])[1]
# Get the file length--- /usr/share/rhn/config_client/rhncfgcli_list.py.orig 2014-05-27
18:16:22.107027400 +0800
+++ /usr/share/rhn/config_client/rhncfgcli_list.py 2014-05-27
18:18:48.678269525 +0800
@@ -16,6 +16,7 @@
from config_common.rhn_log import log_debug, die
import handler_base
+import sys
class Handler(handler_base.HandlerBase):
def run(self):
@@ -32,7 +33,15 @@
maxlen = max(maxlen, len(label)) + 2
print "DoFoS %*s %s" % (maxlen, label, "File")
+ arg_files = []
+ if len(sys.argv) > 2:
+ arg_files = sys.argv[2:len(sys.argv)]
+
for file in files:
+
+ if len(arg_files) and not file[1] in arg_files:
+ continue
+
# checking to see if the filetype is in the 'file' entry,
# and if it is and that type is '1', it is a file
if (len(file) < 3) or file[2] == 1:_______________________________________________
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
