Merge lp:~jameinel/bzr-builddeb/import-scripts-updates into lp:~james-w/bzr-builddeb/import-scripts

[John A Meinel]

[John A Meinel (jameinel)]

This updates icommon.py to add:
1) Create the 'logs/diffs' and 'logs/merges' directories, since they'll be used later.
2) Add 'lp_cache' directory and set the Launchpad instance to use it.

The main win with (2) is that the WADL description of the rest api is something like 1MB, which means that using a cache avoids having to download that whole file everytime I try to run a test import.

I imagine it matters less in production servers (since local bandwidth is cheaper), and LP automatically caches in a temp dir for the duration of a run. But if you are running 3k imports, that is 3GB of content you don't have to download. Especially when there is nothing to do...

[James Westby (james-w)]

> This updates icommon.py to add:
> 1) Create the 'logs/diffs' and 'logs/merges' directories, since they'll be
> used later.

Great.

> 2) Add 'lp_cache' directory and set the Launchpad instance to use it.

Not so great, see bug 459418.

I don't want to turn this on by default with that bug. In fact I used to have
this, but backed it out once I diagnosed the odd errors (bug 404204) I backed
it out.

I'd be happy with something that did this in a way that I could bypass it on
production.

Thanks,

James

review: Needs Fixing

[lp:~jameinel/bzr-builddeb/import-scripts-updates]

312. By John A Meinel on 2010-02-17

Several small updates.
1) handle when madison-lite is not installed more gracefully
2) enable_default_logging() before loading plugins, in case something fails to load.
3) allow passing -Dxxx flags for bzrlib to use for debugging (like -Dhttp, -Derror, and -Dhpss)

[John A Meinel (jameinel)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

James Westby wrote:
> Review: Needs Fixing
>> This updates icommon.py to add:
>> 1) Create the 'logs/diffs' and 'logs/merges' directories, since they'll be
>> used later.
>
> Great.
>
>> 2) Add 'lp_cache' directory and set the Launchpad instance to use it.
>
> Not so great, see bug 459418.
>
> I don't want to turn this on by default with that bug. In fact I used to have
> this, but backed it out once I diagnosed the odd errors (bug 404204) I backed
> it out.
>
> I'd be happy with something that did this in a way that I could bypass it on
> production.
>
> Thanks,
>
> James
>

I'd be happy to provide either an --enable-lp-cache or a
- --disable-lp-cache. Depending on what you want the default to be. For my
interactive testing, I doubt I'd run more than one process, and it
significantly improves the response.

What would you prefer?

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkt8WLIACgkQJdeBCYSNAANEHwCeIQdm6gl3cYbcbClZqagnsAiE
yvYAoM+HA3S69CRRBxDKFC1Yh7ougT7H
=Ci4P
-----END PGP SIGNATURE-----

[lp:~jameinel/bzr-builddeb/import-scripts-updates]

313. By John A Meinel on 2010-02-17

Move the option parsing earlier
this way we can find out about the lp_cache directory before we try to open Launchpad.

314. By John A Meinel on 2010-02-17

add some progress bars for the oh-so-slow getPublishedSources.
Add a bit of caching of opening branches, so that we don't have to
try to re-open the same object hundreds of times.

315. By John A Meinel on 2010-02-18

Use possible transports for file fetching.

This should allow keep-alive. Though it seems there is a bug wrt redirections.

316. By John A Meinel on 2010-02-18

Workaround the redirect bug and implement it ourself.
possible_transports provides all the caching that we would hope to get from
the redirection code, and then some. Testing shows that it does, indeed save
some extra connections. Not all of them, mostly because of the 15s timeout.
(We probe launchpad.net, find we need to download a big file from launchpadlibrarian.net
then spend >15s downloading. The next file reconnects to lp.net but not lpl.net)

=== modified file 'icommon.py'
--- icommon.py	2010-02-11 13:19:55 +0000
+++ icommon.py	2010-02-18 06:21:15 +0000
@@ -58,10 +58,12 @@
 no_lock_returncode = 139
 
 
-for dir in (packages_dir, logs_dir, retry_dir, priority_dir,
-        lists_dir, revids_dir, locks_dir, updates_dir):
-    if not os.path.exists(dir):
-        os.mkdir(dir)
+for directory in (packages_dir, logs_dir, retry_dir, priority_dir, lists_dir,
+                  revids_dir, locks_dir, updates_dir,
+                  debian_diffs_dir, ubuntu_merge_dir,
+                 ):
+    if not os.path.exists(directory):
+        os.mkdir(directory)
 
 urllib_launchpad_base_url = "https+urllib://launchpad.net/"
 urllib_debian_base_url = "http+urllib://ftp.uk.debian.org/debian/"
@@ -84,14 +86,14 @@
 default_ubuntu_merge_source = "squeeze"
 
 
-def get_lp():
+def get_lp(cache=None):
     f = open(lp_creds_file, "rb")
     try:
         creds = Credentials("package-import")
         creds.load(f)
     finally:
         f.close()
-    lp = Launchpad(creds, service_root=SERVICE_ROOT)
+    lp = Launchpad(creds, service_root=SERVICE_ROOT, cache=cache)
     return lp
 
 
@@ -1448,11 +1450,11 @@
             try:
                 revid = old_ubuntu_b.last_revision()
                 to_transport.mkdir('.')
-                dir = old_ubuntu_b.bzrdir.sprout(to_transport.base,
+                abzrdir = old_ubuntu_b.bzrdir.sprout(to_transport.base,
                         revid, possible_transports=possible_transports)
-                ubuntu_b = dir.open_branch()
+                ubuntu_b = abzrdir.open_branch()
                 tag._merge_tags_if_possible(old_ubuntu_b, ubuntu_b)
-                ubuntu_tree = dir.open_workingtree()
+                ubuntu_tree = abzrdir.open_workingtree()
             finally:
                 old_ubuntu_b.unlock()
     if ubuntu_b is None:

=== modified file 'import_package.py'
--- import_package.py	2010-02-12 21:51:28 +0000
+++ import_package.py	2010-02-18 06:21:15 +0000
@@ -1,6 +1,7 @@
 #!/usr/bin/python
 
 import datetime
+import errno
 import operator
 import optparse
 import os
@@ -17,10 +18,15 @@
 
 from launchpadlib.errors import HTTPError
 
-from bzrlib import branch, bzrdir, errors, merge, tag, trace, transport, ui, urlutils
+from bzrlib import branch, bzrdir, debug, errors, merge, tag, trace, transport, ui, urlutils
 from bzrlib.plugin import load_plugins
-from bzrlib.trace import mutter, log_exception_quietly
+from bzrlib.trace import mutter, log_exception_quietly, enable_default_logging
 
+# We may not want to do this, but if there is a failure while loading plugins,
+# it is the only way to find out. One option would be to call
+# 'trace.pop_log_file()' immediately after loading plugins.
+enable_default_logging()
+mutter('import_package started w/ args: %r' % (sys.argv,))
 load_plugins()
 
 from bzrlib.plugins.builddeb import import_dsc
@@ -30,6 +36,20 @@
 
 push_lock = threading.Lock()
 
+parser = optparse.OptionParser()
+parser.add_option("--push-only", dest="push", action="store_true")
+parser.add_option("--no-push", dest="no_push", action="store_true")
+parser.add_option("--check", dest="check", action="store_true")
+parser.add_option("--no-existing", dest="no_existing", action="store_true")
+parser.add_option("--extra-debian", dest="extra_debian")
+parser.add_option("--verbose", dest="verbose", action="store_true")
+parser.add_option("-D", dest="debug_flags", action="append", default=[],
+                  help="Set debug flags, see 'bzr help debug-flags'")
+parser.add_option("--lp-cache", dest="lp_cache", default=None,
+                  help="Set the location of a Launchpadlib cache")
+
+options, args = parser.parse_args()
+
 
 class Killed(Exception):
 
@@ -122,7 +142,7 @@
 ubuntu_base_url = launchpad_base_url + "ubuntu/"
 
 
-lp = icommon.get_lp()
+lp = icommon.get_lp(options.lp_cache)
 ubuntu = lp.distributions['ubuntu']
 u_archive = ubuntu.main_archive
 debian = lp.distributions['debian']
@@ -157,17 +177,32 @@
     publications = icommon.lp_call(icommon.call_with_limited_size,
                 d_archive.getPublishedSources,
                 source_name=package, exact_match=True)
-    for publication in icommon.iterate_collection(publications):
-        assert publication.source_package_name == package
-        release = publication.distro_series.name.lower()
-        pocket = publication.pocket.lower()
-        version = changelog.Version(publication.source_package_version)
-        newp = icommon.PackageToImport(package, version, "debian", release,
-                pocket)
-        vlist.add_if_needed(newp)
+    pb = ui.ui_factory.nested_progress_bar()
+    try:
+        for idx, publication in enumerate(
+                icommon.iterate_collection(publications)):
+            pb.update('getting debian publications', idx, idx)
+            assert publication.source_package_name == package
+            release = publication.distro_series.name.lower()
+            pocket = publication.pocket.lower()
+            version = changelog.Version(publication.source_package_version)
+            newp = icommon.PackageToImport(package, version, "debian", release,
+                    pocket)
+            vlist.add_if_needed(newp)
+            # if idx >= 20:
+            #     break
+    finally:
+        pb.finished()
     update_lists()
-    proc = subprocess.Popen(["/usr/bin/madison-lite", "--mirror", icommon.lists_dir,
-            package], stdout=subprocess.PIPE, preexec_fn=subprocess_setup)
+    try:
+        proc = subprocess.Popen(["/usr/bin/madison-lite", "--mirror", icommon.lists_dir,
+                package], stdout=subprocess.PIPE, preexec_fn=subprocess_setup)
+    except OSError, e:
+        if e.errno in (errno.ENOENT,):
+            sys.stderr.write('You must have "madison-lite" installed.\n'
+                             'try: sudo apt-get install madison-lite.\n')
+            sys.exit(1)
+        raise
     output, errout = proc.communicate()
     assert proc.returncode == 0, "Error running madison-lite"
     for line in output.splitlines():
@@ -210,14 +245,22 @@
     publications = icommon.lp_call(icommon.call_with_limited_size,
                 u_archive.getPublishedSources,
                 source_name=package, exact_match=True)
-    for publication in icommon.iterate_collection(publications):
-        assert publication.source_package_name == package
-        release = publication.distro_series.name.lower()
-        pocket = publication.pocket.lower()
-        version = changelog.Version(publication.source_package_version)
-        newp = icommon.PackageToImport(package, version, "ubuntu", release,
-                pocket)
-        vlist.add_if_needed(newp)
+    pb = ui.ui_factory.nested_progress_bar()
+    try:
+        for idx, publication in enumerate(
+                icommon.iterate_collection(publications)):
+            pb.update('getting ubuntu publications', idx, idx)
+            assert publication.source_package_name == package
+            release = publication.distro_series.name.lower()
+            pocket = publication.pocket.lower()
+            version = changelog.Version(publication.source_package_version)
+            newp = icommon.PackageToImport(package, version, "ubuntu", release,
+                    pocket)
+            vlist.add_if_needed(newp)
+            # if idx >= 20:
+            #     break
+    finally:
+        pb.finished()
     vlist.sort()
     return vlist
 
@@ -270,19 +313,24 @@
         return "debian"
 
 
-def grab_file(location, target_dir):
+def grab_file(location, target_dir, possible_transports=None):
     mutter("fetching %s" % location)
     location_base = urlutils.dirname(location)
     location_file = urlutils.basename(location)
     local_path = os.path.join(target_dir, location_file)
     def get_file(transport):
         return transport.get(location_file)
-    def redirected(transport, e, redirection_notice):
-        target = urlutils.unescape(e.target).encode('utf-8')
-        return transport._redirected_to(e.source, target)
-    location_f = transport.do_catching_redirections(get_file,
-                        transport.get_transport(location_base),
-                        redirected)
+    def redirected(t, e, redirection_notice):
+        # _redirected_to has a bug that it doesn't support possible_transports,
+        # so we just call get_transport directly, we know we are just getting a
+        # file, so we don't have to try too hard.
+        base, fname = urlutils.split(e.target)
+        t2 = transport.get_transport(base,
+                                     possible_transports=possible_transports)
+        return t2
+    t = transport.get_transport(location_base,
+                                possible_transports=possible_transports)
+    location_f = transport.do_catching_redirections(get_file, t, redirected)
     try:
         local_f = open(local_path, "wb")
         try:
@@ -293,8 +341,8 @@
         location_f.close()
 
 
-def dget(dsc_location, target_dir):
-    grab_file(dsc_location, target_dir)
+def dget(dsc_location, target_dir, possible_transports=None):
+    grab_file(dsc_location, target_dir, possible_transports=possible_transports)
     local_dsc_path = os.path.join(target_dir,
         urlutils.basename(dsc_location))
     dsc_f = open(local_dsc_path)
@@ -305,7 +353,7 @@
             name = file_info['name']
             # TODO: md5 check
             grab_file(urlutils.join(urlutils.dirname(dsc_location), name),
-                target_dir)
+                target_dir, possible_transports=possible_transports)
     finally:
         dsc_f.close()
     return local_dsc_path
@@ -510,7 +558,8 @@
         extract_upstream_branch(update_db, upstream_dir)
         dl_dir = tempfile.mkdtemp()
         try:
-            local_dsc_path = dget(importp.get_url(), temp_dir)
+            local_dsc_path = dget(importp.get_url(), temp_dir,
+                                  possible_transports=possible_transports)
             update_db.import_package(local_dsc_path,
                     use_time_from_changelog=True)
         finally:
@@ -531,19 +580,26 @@
     unimported_versions = icommon.ImportList()
     imported_suites = set()
     have_imported_old_debian = False
+    last_target_info = None
     for importp in reversed(all_versions.plist):
         suite = icommon.make_suite(importp.release, importp.pocket)
         if (importp.distro, suite) in imported_suites:
             continue
         target_branch_path = os.path.join(temp_dir, suite)
-        try:
-            target_branch = branch.Branch.open(target_branch_path,
-                    possible_transports=possible_transports)
-            has_version = True
-        except errors.NotBranchError:
-            target_branch = bstore.get_branch(importp,
-                    possible_transports=possible_transports)
-            has_version = target_branch is not None
+        if last_target_info is not None:
+            if target_branch_path == last_target_info[0]:
+                target_branch = last_target_info[1]
+                has_version = target_branch is not None
+        else:
+            try:
+                target_branch = branch.Branch.open(target_branch_path,
+                        possible_transports=possible_transports)
+                has_version = True
+            except errors.NotBranchError:
+                target_branch = bstore.get_branch(importp,
+                        possible_transports=possible_transports)
+                has_version = target_branch is not None
+        last_target_info = (target_branch_path, target_branch)
         if has_version:
             db = import_dsc.DistributionBranch(target_branch, target_branch)
             has_version = db.has_version(importp.version)
@@ -764,10 +820,11 @@
             newdir = format.initialize_on_transport(to_transport)
             repo = newdir.create_repository(shared=True)
             repo.set_make_working_trees(True)
+            mutter("finding all versions of %s" % (package,))
             versions = get_versions(package, extra_debian=extra_debian)
             assert len(versions.plist) > 0
             versions.reverse()
-            mutter(str(versions))
+            mutter("found %d versions: %s" % (len(versions.plist), versions))
             possible_transports = []
             versions = find_unimported_versions(temp_dir, versions, package,
                     revid_db, bstore, possible_transports=possible_transports)
@@ -818,19 +875,13 @@
         f.close()
 
 
-parser = optparse.OptionParser()
-parser.add_option("--push-only", dest="push", action="store_true")
-parser.add_option("--no-push", dest="no_push", action="store_true")
-parser.add_option("--check", dest="check", action="store_true")
-parser.add_option("--no-existing", dest="no_existing", action="store_true")
-parser.add_option("--extra-debian", dest="extra_debian")
-parser.add_option("--verbose", dest="verbose", action="store_true")
-
-options, args = parser.parse_args()
-
 if options.verbose:
     trace.set_verbosity_level(1)
     trace.push_log_file(sys.stderr)
+if sys.stderr.isatty():
+    # Don't create a progress bar if we aren't actually in a terminal
+    ui.ui_factory = ui.make_ui_for_terminal(sys.stdin, sys.stdout, sys.stderr)
+debug.debug_flags.update(options.debug_flags)
 
 
 if len(args) < 1: