Hi Xavier On Friday 14 Nov 2008, Xavier Luthi wrote: > I've prepared a new package version for apt-proxy. It's currently an > NMU but of course, if you've time, you can upload it as a new non-NMU > revision of the package.
Thanks a lot for making your changes. Do you have an Alioth account? If so I'll add you to the project so you can check changes into SVN. I'm too busy to make an upload before the end of next week, so please go ahead with an upload if you want to do it before then. > You'll find attached the corresponding debdiff, the package itself > can be downloaded here: > - URL: http://mentors.debian.net/debian/pool/main/a/apt-proxy I have some comments on your changes. > + # see bug #285770 > + if len(value) == 0: > + return None > - raise ConfigError("Configuration parse error: [%s] %s" % > (section, option)) + # see bug #285770 > + #raise ConfigError("Configuration parse error: [%s] %s" % > (section, option)) + return None > value = self.parseConfigValue(config, DEFAULTSECT, name, > default, getmethod) + # see bug #285770 > + if value == None: > + value = default > + # end of bug #285770 These changes feel ugly to me. An empty value, such as 'timeout =' is an error in the configuration and I think you should only use the default value if the line is not there in the configuration at all. What about handling the ConfigError exception and giving a proper error message instead? > - self.worker = FileVerifierProcess(self, '/usr/bin/bunzip2', > '--test', self.path) > + self.worker = FileVerifierProcess(self, > '/bin/bunzip2', '--test', self.path) else: Are we sure that bunzip2 will always be found here? It might be better to use the system path instead of a hardcoded absolute path. > +## change process priority as requested in http://bugs.debian.org/275658 > +# and do that silent > +renice 5 $$ >/dev/null I think this should be configurable in /etc/default/apt-proxy so that the system administrator can turn this off if they need to. > +process=$(basename $application) > + sleep 1 # for some startup time and then check > + ps -eo cmd | grep -q ^$process Can you use ps -u $user instead of ps -e, to only check processes with the aptproxy user? I am very happy with all of your other changes - thanks! Chris -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
