On Wed, 2011-07-06 at 18:25 -0700, Dale Curtis wrote: > 1. Fix missing default values for PACKAGER/minimum_free_space and > PACKAGER/custom_max_age. These prevented the packager from running on a > fresh installation. > > 2. Add values mentioned in 1 to global_config for easier discovery. > > 3. Modify trim_custom_directories behavior to use the default value only > when one is not provided to the function. > > 4. Various spelling and style cleanup.
Ok, I made some comments on the gerrit issue, I've applied this change as is: http://autotest.kernel.org/changeset/5492 Great stuff, thanks! I'll take care of the cosmetics of utils/packager.py messages on a later patch. > Not completely the same, but similar change review URL: > > http://gerrit.chromium.org/gerrit/#change,3701 > Signed-off-by: Dale Curtis <[email protected]> > --- > client/common_lib/base_packages.py | 30 ++++++++++++------------ > global_config.ini | 8 ++++++ > utils/packager.py | 44 > ++++++++++++++++++------------------ > 3 files changed, 45 insertions(+), 37 deletions(-) > > diff --git a/client/common_lib/base_packages.py > b/client/common_lib/base_packages.py > index 2a60b07..cf71949 100644 > --- a/client/common_lib/base_packages.py > +++ b/client/common_lib/base_packages.py > @@ -5,8 +5,7 @@ upload and remove packages. Site specific extensions to any > of these methods > should inherit this class. > """ > > -import re, os, sys, traceback, subprocess, shutil, time, traceback, urlparse > -import fcntl, logging > +import fcntl, logging, os, re, shutil > from autotest_lib.client.bin import os_dep > from autotest_lib.client.common_lib import error, utils, global_config > > @@ -74,13 +73,13 @@ def create_directory(repo): > > def check_diskspace(repo, min_free=None): > # Note: 1 GB = 10**9 bytes (SI unit). > - if not min_free: > + if min_free is None: > min_free = global_config.global_config.get_config_value('PACKAGES', > > 'minimum_free_space', > - type=int) > + type=int, > default=1) > try: > df = repo_run_command(repo, > - 'df -PB %d . | tail -1' % 10**9).stdout.split() > + 'df -PB %d . | tail -1' % 10 ** > 9).stdout.split() > free_space_gb = int(df[3]) > except Exception, e: > raise error.RepoUnknownError('Unknown Repo Error: %s' % e) > @@ -98,12 +97,13 @@ def check_write(repo): > raise error.RepoWriteError('Unable to write to ' + repo) > > > -def trim_custom_directories(repo, older_than_days=40): > +def trim_custom_directories(repo, older_than_days=None): > if not repo: > return > - older_than_days = > global_config.global_config.get_config_value('PACKAGES', > - 'custom_max_age', > - type=int) > + > + if older_than_days is None: > + older_than_days = global_config.global_config.get_config_value( > + 'PACKAGES', 'custom_max_age', type=int, default=40) > cmd = 'find . -type f -atime +%s -exec rm -f {} \;' % older_than_days > repo_run_command(repo, cmd, ignore_status=True) > > @@ -309,7 +309,7 @@ class BasePackageManager(object): > ''' > from autotest_lib.server import subcommand > if not custom_repos: > - # Not all package types necessairly require or allow custom repos > + # Not all package types necessarily require or allow custom repos > try: > custom_repos = global_config.global_config.get_config_value( > 'PACKAGES', 'custom_upload_location').split(',') > @@ -325,8 +325,8 @@ class BasePackageManager(object): > if not custom_repos: > return > > - results = subcommand.parallel_simple(trim_custom_directories, > - custom_repos, log=False, ) > + subcommand.parallel_simple(trim_custom_directories, custom_repos, > + log=False) > > > def install_pkg(self, name, pkg_type, fetch_dir, install_dir, > @@ -400,7 +400,7 @@ class BasePackageManager(object): > def fetch_pkg(self, pkg_name, dest_path, repo_url=None, > use_checksum=False): > ''' > Fetch the package into dest_dir from repo_url. By default repo_url > - is None and the package is looked in all the repostories specified. > + is None and the package is looked in all the repositories specified. > Otherwise it fetches it from the specific repo_url. > pkg_name : name of the package (ex: test-sleeptest.tar.bz2, > dep-gcc.tar.bz2, kernel.1-1.rpm) > @@ -666,7 +666,7 @@ class BasePackageManager(object): > # The packages checksum file does not exist locally. > # See if it is present in the repositories. > self.fetch_pkg(CHECKSUM_FILE, checksum_path) > - except error.PackageFetchError, e: > + except error.PackageFetchError: > # This should not happen whilst fetching a package..if a > # package is present in the repository, the corresponding > # checksum file should also be automatically present. This > @@ -702,7 +702,7 @@ class BasePackageManager(object): > checksum_path = self._get_checksum_file_path() > self._checksum_dict = checksum_dict.copy() > checksum_contents = '\n'.join(checksum + ' ' + pkg_name > - for pkg_name,checksum in > + for pkg_name, checksum in > checksum_dict.iteritems()) > # Write the checksum file back to disk > self._run_command('echo "%s" > %s' % (checksum_contents, > diff --git a/global_config.ini b/global_config.ini > index 51f660a..1f0a76a 100644 > --- a/global_config.ini > +++ b/global_config.ini > @@ -125,4 +125,12 @@ require_atfork_module: False > use_sshagent_with_paramiko: True > > [PACKAGES] > +# Max age of packages in days. All packages older than this will be removed > from > +# upload_location when the packager is run. > +custom_max_age: 40 > +# Minimum amount of disk space, in gigabytes, required for packaging. > +minimum_free_space: 1 > +serve_packages_from_autoserv: True > +# Location to store packages in. > +upload_location: > serve_packages_from_autoserv: True > diff --git a/utils/packager.py b/utils/packager.py > index 2a23534..edc7a5b 100755 > --- a/utils/packager.py > +++ b/utils/packager.py > @@ -4,7 +4,7 @@ > Utility to upload or remove the packages from the packages repository. > """ > > -import logging, os, sys, optparse, socket, tempfile, shutil > +import logging, optparse, os, shutil, sys, tempfile > import common > from autotest_lib.client.common_lib import utils as client_utils > from autotest_lib.client.common_lib import global_config, error > @@ -77,10 +77,10 @@ def process_packages(pkgmgr, pkg_type, pkg_names, src_dir, > names = [p.strip() for p in pkg_names.split(',')] > for name in names: > print "Processing %s ... " % name > - if pkg_type=='client': > + if pkg_type == 'client': > pkg_dir = src_dir > - exclude_string = get_exclude_string(pkg_dir) > - elif pkg_type=='test': > + exclude_string = get_exclude_string(pkg_dir) > + elif pkg_type == 'test': > # if the package is a test then look whether it is in > client/tests > # or client/site_tests > pkg_dir = os.path.join(get_test_dir(name, src_dir), name) > @@ -117,10 +117,10 @@ def tar_packages(pkgmgr, pkg_type, pkg_names, src_dir, > temp_dir): > names = [p.strip() for p in pkg_names.split(',')] > for name in names: > print "Processing %s ... " % name > - if pkg_type=='client': > + if pkg_type == 'client': > pkg_dir = src_dir > - exclude_string = get_exclude_string(pkg_dir) > - elif pkg_type=='test': > + exclude_string = get_exclude_string(pkg_dir) > + elif pkg_type == 'test': > # if the package is a test then look whether it is in > client/tests > # or client/site_tests > pkg_dir = os.path.join(get_test_dir(name, src_dir), name) > @@ -139,8 +139,6 @@ def tar_packages(pkgmgr, pkg_type, pkg_names, src_dir, > temp_dir): > > def process_all_packages(pkgmgr, client_dir, remove=False): > """Process a full upload of packages as a directory upload.""" > - test_dir = os.path.join(client_dir, "tests") > - site_test_dir = os.path.join(client_dir, "site_tests") > dep_dir = os.path.join(client_dir, "deps") > prof_dir = os.path.join(client_dir, "profilers") > # Directory where all are kept > @@ -183,7 +181,7 @@ def process_all_packages(pkgmgr, client_dir, > remove=False): > pkgmgr.upload_pkg(temp_dir) > client_utils.run('rm -rf ' + temp_dir) > else: > - process_packages(pkgmgr, 'test', tests, client_dir,remove=remove) > + process_packages(pkgmgr, 'test', tests, client_dir, remove=remove) > process_packages(pkgmgr, 'test', site_tests, client_dir, > remove=remove) > process_packages(pkgmgr, 'client', 'autotest', client_dir, > remove=remove) > @@ -226,8 +224,15 @@ def main(): > type=list, default=[]) > upload_paths = c.get_config_value('PACKAGES', 'upload_location', > type=list, default=[]) > + > + if options.repo: > + upload_paths.append(options.repo) > + > # Having no upload paths basically means you're not using packaging. > - if len(upload_paths) == 0: > + if not upload_paths: > + print("No upload locations found. Please set upload_location under" > + " PACKAGES in the global_config.ini or provide a location > using" > + " the --repository option.") > return > > client_dir = os.path.join(autotest_dir, "client") > @@ -239,26 +244,21 @@ def main(): > dep_dir = os.path.join(client_dir, "deps") > prof_dir = os.path.join(client_dir, "profilers") > > - if len(args)==0 or args[0] not in ['upload','remove']: > + if len(args) == 0 or args[0] not in ['upload', 'remove']: > print("Either 'upload' or 'remove' needs to be specified " > "for the package") > sys.exit(0) > > - if args[0]=='upload': > - remove_flag=False > - elif args[0]=='remove': > - remove_flag=True > + if args[0] == 'upload': > + remove_flag = False > + elif args[0] == 'remove': > + remove_flag = True > else: > # we should not be getting here > assert(False) > > - if options.repo: > - upload_path_list = [options.repo] > - else: > - upload_path_list = upload_paths > - > pkgmgr = packages.PackageManager(autotest_dir, repo_urls=repo_urls, > - upload_paths=upload_path_list, > + upload_paths=upload_paths, > run_function_dargs={'timeout':600}) > > if options.all: _______________________________________________ Autotest mailing list [email protected] http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
