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
