[Bps-public-commit] SD branch, master, updated. 0.74-96-ga0ee3de

Christine Spang spang at bestpractical.com
Wed Mar 2 22:19:40 EST 2011


The branch, master has been updated
       via  a0ee3de1cb6cbe63028d71f8b4c6642006175034 (commit)
       via  af61a2e2949c1276ffd08d838fad722ac1f9a7aa (commit)
       via  7c19d50d8a845ddfb69feffd6b8fcebde3235bbe (commit)
       via  c7632007cf1a70357721b70a43528394ca88da94 (commit)
       via  26ccda4ee5bc6039e8e98262c18a6906091f1009 (commit)
       via  d8b3d5482f915524517e02929f05a715c3f166fe (commit)
       via  39d5ee561f69dd5ee23d4a5b9de3be1bb4a40ff2 (commit)
       via  025987c44b483c6c979028ac146d36b6d81dbede (commit)
       via  ba6f8b438b25f5f6452ef94542eb6ea25e5c9c4c (commit)
       via  abeed5523c2099dca690d6bdf1f857e367fd6d17 (commit)
       via  f9000efe4b8914ce1e9281cf70da84f68d70da81 (commit)
       via  e9d81feb6d91768b5731d76eb21805ef152c6923 (commit)
       via  5b08c2f0cfcbc0949715d1e42db21a8fffe95c71 (commit)
       via  edf943bfb80dd85e02ede4e3c0e8f40b07eefbae (commit)
       via  34411d982b4826b90055d94585ab1f5fb1deef5a (commit)
       via  7f56cb5f7c42e9124f058eb95b40da4861616d5c (commit)
       via  2103f4c08a04f442868c46f57c13f17c32b559e2 (commit)
       via  639d4e5887d99836b2a913f7b3a4d4f3b4c95506 (commit)
       via  cec75345e23f7b77be74d6e037f5452d5cc1dda5 (commit)
       via  e815d09d98fe6fab4a04ec7668dc5a1b80eeac2a (commit)
      from  97412a897ecc0708e9307a2fff6c7950334cfd5f (commit)

Summary of changes:
 Makefile.PL                              |    2 +
 lib/App/SD/CLI/Command/Help/Config.pm    |    6 +
 lib/App/SD/CLI/Command/Ticket/Search.pm  |    8 +-
 lib/App/SD/ForeignReplica.pm             |  200 ++++++++++++++++++++++-------
 lib/App/SD/ForeignReplica/PullEncoder.pm |    9 +-
 lib/App/SD/Replica/gcode.pm              |   72 +++++++----
 lib/App/SD/Replica/github.pm             |   45 ++++---
 lib/App/SD/Replica/github/PullEncoder.pm |    3 +-
 lib/App/SD/Replica/hm.pm                 |   81 +++++++++---
 lib/App/SD/Replica/hm/PushEncoder.pm     |   41 +++----
 lib/App/SD/Replica/rt.pm                 |   42 +++----
 lib/App/SD/Replica/trac.pm               |   60 +++++----
 lib/App/SD/Replica/trac/PullEncoder.pm   |   28 +++--
 lib/App/SD/Replica/trac/PushEncoder.pm   |   25 +++--
 t/sd-hm/basics.t                         |    1 +
 t/sd-rt/basic.t                          |    2 +-
 t/server.t                               |    3 +-
 17 files changed, 406 insertions(+), 222 deletions(-)

- Log -----------------------------------------------------------------
commit e815d09d98fe6fab4a04ec7668dc5a1b80eeac2a
Author: Christine Spang <christine at debian.org>
Date:   Mon Feb 21 10:55:10 2011 -0500

    english grammar++

diff --git a/lib/App/SD/ForeignReplica/PullEncoder.pm b/lib/App/SD/ForeignReplica/PullEncoder.pm
index c5a4232..1781350 100644
--- a/lib/App/SD/ForeignReplica/PullEncoder.pm
+++ b/lib/App/SD/ForeignReplica/PullEncoder.pm
@@ -85,7 +85,8 @@ sub transcode_history {
         my $changeset = $self->transcode_one_txn( $txn, $initial_state, $final_state );
         next unless $changeset && $changeset->has_changes;
 
-        # the changesets are older than the ones that came before, so they goes first
+        # the changesets are older than the ones that came before, so they go
+        # first
         unshift @changesets, $changeset;
     }
     return ( $last_modified, \@changesets );

commit cec75345e23f7b77be74d6e037f5452d5cc1dda5
Author: Christine Spang <christine at debian.org>
Date:   Mon Feb 21 10:59:50 2011 -0500

    sort foreign changesets to be integrated by original_sequence_no, not creation time
    
    When creating changesets, we first fetch the set of tickets and then
    fetch the history for each ticket, creating changesets for each part of
    the ticket's history..
    
    If we sort the resulting changesets by creation time, they will end up
    being interleaved by ticket, which may result in some changesets being
    skipped that shouldn't be.

diff --git a/lib/App/SD/ForeignReplica/PullEncoder.pm b/lib/App/SD/ForeignReplica/PullEncoder.pm
index 1781350..88caf16 100644
--- a/lib/App/SD/ForeignReplica/PullEncoder.pm
+++ b/lib/App/SD/ForeignReplica/PullEncoder.pm
@@ -35,7 +35,9 @@ sub run {
         ( $last_modified, $changesets ) = $self->transcode_ticket( $ticket, $last_modified );
         unshift @changesets, @$changesets;
     }
-    return [ sort { App::SD::Util::string_to_datetime($a->created) <=> App::SD::Util::string_to_datetime( $b->created) } @changesets];
+    my $sorted_changesets = [ sort {
+        $a->original_sequence_no <=> $b->original_sequence_no } @changesets ];
+    return $sorted_changesets;
 }
 
 sub ticket_last_modified { undef}

commit 639d4e5887d99836b2a913f7b3a4d4f3b4c95506
Author: Christine Spang <christine at debian.org>
Date:   Mon Feb 21 12:28:54 2011 -0500

    add config option ticket.<prop>.sort-undef-last
    
    Sometimes, like with, for example due dates, we may want items with no set
    value to sort *after* any tickets with existing due dates.

diff --git a/lib/App/SD/CLI/Command/Ticket/Search.pm b/lib/App/SD/CLI/Command/Ticket/Search.pm
index de4c776..a7e39f7 100644
--- a/lib/App/SD/CLI/Command/Ticket/Search.pm
+++ b/lib/App/SD/CLI/Command/Ticket/Search.pm
@@ -50,10 +50,16 @@ override run => sub {
     if ( $self->has_arg('sort') && $self->arg('sort')
             && ( $self->arg('sort') ne 'none' ) ) {
 
+        my $sort_prop = $self->arg('sort');
+
+        my $sort_undef_last = $self->app_handle->config->get(
+            key => $self->type . ".$sort_prop.sort-undef-last" );
+
         $self->sort_routine(
             sub {
                 my $records = shift;
-                return $self->sort_by_prop( $self->arg('sort') => $records );
+                return $self->sort_by_prop( $sort_prop, $records,
+                                            $sort_undef_last );
             }
         );
     }

commit 2103f4c08a04f442868c46f57c13f17c32b559e2
Author: Christine Spang <christine at debian.org>
Date:   Mon Feb 21 12:37:42 2011 -0500

    document ticket.<prop>.sort-undef-last

diff --git a/lib/App/SD/CLI/Command/Help/Config.pm b/lib/App/SD/CLI/Command/Help/Config.pm
index a7e0e82..a39dc73 100644
--- a/lib/App/SD/CLI/Command/Help/Config.pm
+++ b/lib/App/SD/CLI/Command/Help/Config.pm
@@ -73,6 +73,12 @@ by configuration file section):
       Bug property to determine order of display when displaying lists of
       tickets. (Can be any property; will be compared lexically.)
 
+    ticket.<prop>.sort-undef-last = true
+      When sorting on <prop>, setting this will make tickets where
+      the property is undefined sort *after* any records where the
+      property *is* defined. (The default is the opposite.)
+      Useful for e.g. due dates.
+
     ticket.default-group = milestone
       Bug property to group tickets by when displaying lists of tickets. (Can
       be any property.)

commit 7f56cb5f7c42e9124f058eb95b40da4861616d5c
Author: Christine Spang <christine at debian.org>
Date:   Mon Feb 21 15:48:41 2011 -0500

    hiveminder: loop username/password prompting until login successful & clean up Net::Jifty error messages

diff --git a/Makefile.PL b/Makefile.PL
index 2c67aff..a932d69 100644
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -14,6 +14,7 @@ requires('HTML::TreeBuilder' => '4.1');
 requires('DateTime::Format::Natural');
 requires('HTML::Tree');
 requires('URI::file');
+requires('Try::Tiny' => '0.02');
 
 build_requires('Test::Script::Run' => '0.02');
 
diff --git a/lib/App/SD/ForeignReplica.pm b/lib/App/SD/ForeignReplica.pm
index 1916d4d..1020ad6 100644
--- a/lib/App/SD/ForeignReplica.pm
+++ b/lib/App/SD/ForeignReplica.pm
@@ -15,6 +15,37 @@ has uuid => (
     }
 );
 
+=head2 save_username_and_token( $username, $token )
+
+Saves the given username and token to the replica-specific config file,
+so the user doesn't have to enter it every time.
+
+=cut
+
+sub save_username_and_token {
+    my ($self, $username, $password) = @_;
+
+    # make sure replica is initialized
+    $self->app_handle->handle->initialize;
+
+    my $replica_username_key = 'replica.' . $self->scheme . ":" . $self->{url} . '.username';
+    my $replica_token_key    = 'replica.' . $self->scheme . ":" . $self->{url} . '.secret_token';
+
+    if ( !$self->app_handle->config->get( key => $replica_username_key ) ) {
+        print "Setting replica's username and token in the config file";
+        $self->app_handle->config->group_set(
+            $self->app_handle->config->replica_config_file,
+         [ {
+            key      => $replica_username_key,
+            value    => $username,
+         }, {
+            key      => $replica_token_key,
+            value    => $password,
+         } ],
+        );
+    }
+}
+
 
 sub integrate_changeset {
     my $self = shift;
diff --git a/lib/App/SD/Replica/hm.pm b/lib/App/SD/Replica/hm.pm
index 8dccb18..f055aec 100644
--- a/lib/App/SD/Replica/hm.pm
+++ b/lib/App/SD/Replica/hm.pm
@@ -6,6 +6,7 @@ use URI;
 use Memoize;
 use Prophet::ChangeSet;
 use File::Temp 'tempdir';
+use Try::Tiny
 
 has hm               => ( isa => 'Net::Jifty', is => 'rw' );
 has remote_url       => ( isa => 'Str',        is => 'rw' );
@@ -50,27 +51,49 @@ sub BUILD {
     }
     $self->remote_url("$uri");
 
-    ( $username, $password )
-        = $self->prompt_for_login(
-            uri      => $uri,
-            username => $username,
-        ) unless $password;
-
-    $self->foreign_username($username) if ($username);
+    my $login_successful = 0;
+
+    while (!$login_successful) {
+        ( $username, $password )
+            = $self->prompt_for_login(
+                uri      => $uri,
+                username => $username,
+                # remind the user that hiveminder logins are email addresses
+                username_prompt => sub {
+                    my $uri = shift;
+                    return "Login email for $uri: ";
+                },
+            ) unless $password;
+
+        $self->foreign_username($username) if ($username);
+
+        try {
+            $self->hm(
+                Net::Jifty->new(
+                    site        => $self->remote_url,
+                    cookie_name => 'JIFTY_SID_HIVEMINDER',
+
+                    email    => $username,
+                    password => $password
+                )
+            );
+            $login_successful = 1;
+        } catch {
+            # Net::Jifty uses Carp::confess to deal with login problems :(
+            ($username, $password) = (undef, undef);
+            my $error_message = (split /\n/, $_)[0];
+            $error_message =~ s/ at .* line [0-9]+$//;
+            warn "\n$error_message\n\n";
+        };
+    }
 
     if ($props) {
         my %props = split /=|;/, $props;
         $self->props( \%props );
     }
-    $self->hm(
-        Net::Jifty->new(
-            site        => $self->remote_url,
-            cookie_name => 'JIFTY_SID_HIVEMINDER',
-
-            email    => $username,
-            password => $password
-        )
-    );
+
+    # only save username/password if login was successful
+    $self->save_username_and_token( $username, $password );
 }
 
 =head2 _uuid_url

commit 34411d982b4826b90055d94585ab1f5fb1deef5a
Author: Christine Spang <christine at debian.org>
Date:   Mon Feb 21 17:26:03 2011 -0500

    fix t/server.t

diff --git a/t/server.t b/t/server.t
index 9682c57..05cef0f 100644
--- a/t/server.t
+++ b/t/server.t
@@ -89,11 +89,10 @@ $ua->title_like(qr/^(\d+): Test ticket/);
 
 sub start_server {
     my $server_cli = App::SD::CLI->new();
-    my $s          = App::SD::Server->new();
+    my $s          = App::SD::Server->new( app_handle => $server_cli->app_handle );
     unshift @App::SD::Server::ISA, 'Test::HTTP::Server::Simple';
     $server_cli->handle()->initialize;
     $s->port( int( rand(10000) + 1024 ) );
-    $s->app_handle( $server_cli->app_handle );
     my $url_root = $s->started_ok("start up my web server");
     return $url_root;
 }

commit edf943bfb80dd85e02ede4e3c0e8f40b07eefbae
Author: Christine Spang <christine at debian.org>
Date:   Mon Feb 21 17:47:17 2011 -0500

    add missing Path::Class hm sync dep

diff --git a/Makefile.PL b/Makefile.PL
index a932d69..a447a87 100644
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -30,6 +30,7 @@ feature 'Hiveminder sync' => (
     -default => 0,
     'Net::Jifty' => 0.09,
     'Email::Address' => 0,
+    'Path::Class' => 0,
 );
 
 recommends 'Net::Jifty';

commit 5b08c2f0cfcbc0949715d1e42db21a8fffe95c71
Author: Christine Spang <christine at debian.org>
Date:   Wed Feb 23 00:33:14 2011 -0500

    fix hiveminder sync

diff --git a/lib/App/SD/Replica/hm.pm b/lib/App/SD/Replica/hm.pm
index f055aec..7cb707b 100644
--- a/lib/App/SD/Replica/hm.pm
+++ b/lib/App/SD/Replica/hm.pm
@@ -6,7 +6,8 @@ use URI;
 use Memoize;
 use Prophet::ChangeSet;
 use File::Temp 'tempdir';
-use Try::Tiny
+use Try::Tiny;
+use Carp;
 
 has hm               => ( isa => 'Net::Jifty', is => 'rw' );
 has remote_url       => ( isa => 'Str',        is => 'rw' );
@@ -96,6 +97,25 @@ sub BUILD {
     $self->save_username_and_token( $username, $password );
 }
 
+sub request_failed {
+    my ($self, $response) = @_;
+
+    return defined($response->{success}) && $response->{success} == 0;
+}
+
+sub decode_error {
+    my $self   = shift;
+    my $status = shift;
+    my $msg    = '';
+    $msg .= $status->{'error'} if defined $status->{'error'};
+    if ( $status->{'field_errors'} ) {
+        while ( my ( $k, $v ) = each %{ $status->{'field_errors'} } ) {
+            $msg .= "field '$k' - '$v'\n";
+        }
+    }
+    return $msg;
+}
+
 =head2 _uuid_url
 
 Return the replica's UUID
@@ -138,7 +158,11 @@ sub _user_info {
     my $value = shift;
     return undef unless defined $value;
     my $status = $self->hm->search('User', $key => $value);
-    die $status->{'error'} unless $status->[0]->{'id'};
+    unless ( $status->[0]->{'id'} ) {
+        # some weird error
+        warn "fatal error in _user_info\n";
+        Carp::confess;
+    }
     return $status->[0];
 }
 memoize '_user_info';
diff --git a/lib/App/SD/Replica/hm/PushEncoder.pm b/lib/App/SD/Replica/hm/PushEncoder.pm
index 9a82343..98378da 100644
--- a/lib/App/SD/Replica/hm/PushEncoder.pm
+++ b/lib/App/SD/Replica/hm/PushEncoder.pm
@@ -59,11 +59,13 @@ sub integrate_ticket_create {
     }
 
     my $task = $self->sync_source->hm->create( 'Task', %args );
-    unless ( $task->{'success'} ) {
-        die "Couldn't create a task: " . $self->decode_error($task);
+    # a successful create just returns the task's data, and doesn't
+    # have a 'success' member at all
+    if ( $self->sync_source->request_failed($task) ) {
+        die "Couldn't create a task: " . $self->sync_source->decode_error($task);
     }
 
-    my $tid = $task->{content}->{id};
+    my $tid = $task->{id};
 
     if (@requesters) {
         my $email = $self->comment_as_email(
@@ -77,7 +79,7 @@ sub integrate_ticket_create {
             message => $email->as_string,
         );
         warn "Couldn't add a comment on the recently created HM task"
-            unless $status->{'success'};
+            if $self->sync_source->request_failed($status);
     }
 
     my $txns = $self->sync_source->hm->search( 'TaskTransaction', task_id => $tid );
@@ -92,19 +94,6 @@ sub integrate_ticket_create {
     return $tid;
 }
 
-sub decode_error {
-    my $self   = shift;
-    my $status = shift;
-    my $msg    = '';
-    $msg .= $status->{'error'} if defined $status->{'error'};
-    if ( $status->{'field_errors'} ) {
-        while ( my ( $k, $v ) = each %{ $status->{'field_errors'} } ) {
-            $msg .= "field '$k' - '$v'\n";
-        }
-    }
-    return $msg;
-}
-
 sub integrate_comment {
     my $self = shift;
     my ( $change, $changeset )
@@ -121,9 +110,9 @@ sub integrate_comment {
         task_id => $ticket_id,
         message => $email->as_string,
     );
-    return $status->{'content'}{'id'} if $status->{'success'};
+    return $status->{'id'} unless $self->sync_source->request_failed($status);
 
-    die "Couldn't integrate comment: " . $self->decode_error($status);
+    die "Couldn't integrate comment: " . $self->sync_source->decode_error($status);
 }
 
 sub integrate_ticket_update {
@@ -172,9 +161,9 @@ sub integrate_ticket_update {
             id => $tid,
             %args,
         );
-        die "Couldn't integrate ticket update: " . $self->decode_error($status)
-            unless $status->{'success'};
-        $txn_id = $status->{'content'}{'id'};
+        die "Couldn't integrate ticket update: " . $self->sync_source->decode_error($status)
+            if $self->sync_source->request_failed($status);
+        $txn_id = $status->{'id'};
     }
 
     if (@new_requestors) {
@@ -212,8 +201,8 @@ sub record_comment {
         message => $email->as_string,
     );
     warn "Couldn't add a comment on the recently created HM task"
-        unless $status->{'success'};
-    return $status->{'content'}{'id'};
+        if $self->sync_source->request_failed($status);
+    return $status->{'id'};
 }
 
 sub integrate_attachment {
@@ -241,9 +230,9 @@ sub integrate_attachment {
         task_id => $ticket_id,
         %props,
     );
-    return $status->{'content'}{'id'} if $status->{'success'};
+    return $status->{'id'} unless $self->sync_source->request_failed($status);
 
-    die "Couldn't integrate attachment: " . $self->decode_error($status);
+    die "Couldn't integrate attachment: " . $self->sync_source->decode_error($status);
 }
 
 sub _recode_props_for_create {
diff --git a/t/sd-hm/basics.t b/t/sd-hm/basics.t
index 9873144..8d87d0e 100644
--- a/t/sd-hm/basics.t
+++ b/t/sd-hm/basics.t
@@ -22,6 +22,7 @@ eval 'use BTDT::Test; 1;' or die "$@";
 
 my $server = BTDT::Test->make_server;
 my $URL    = $server->started_ok;
+# specify username and password in URL
 $URL =~ s|http://|http://onlooker\@example.com:something@|;
 
 my $GOODUSER;

commit e9d81feb6d91768b5731d76eb21805ef152c6923
Author: Christine Spang <christine at debian.org>
Date:   Thu Feb 24 22:46:30 2011 -0500

    fix t/sd-rt/basic.t

diff --git a/t/sd-rt/basic.t b/t/sd-rt/basic.t
index e2bd745..a77f5da 100644
--- a/t/sd-rt/basic.t
+++ b/t/sd-rt/basic.t
@@ -244,7 +244,7 @@ ok( $ret, $out );
 ( $ret, $out, $err )
     = run_script( 'sd', [ 'ticket', 'show', '--verbose', '--id', $yatta_id ] );
 
-like( $out, qr/"cc"\s+set\s+to\s+"hiro\@example.com"/ );
+like( $out, qr/cc:\s+set\s+to\s+hiro\@example.com/ );
 
 diag("resolve and comment on a ticket");
 

commit f9000efe4b8914ce1e9281cf70da84f68d70da81
Author: Christine Spang <christine at debian.org>
Date:   Sun Feb 27 00:24:45 2011 -0500

    misc style & formatting cleanup

diff --git a/lib/App/SD/ForeignReplica.pm b/lib/App/SD/ForeignReplica.pm
index 1020ad6..2d8fba1 100644
--- a/lib/App/SD/ForeignReplica.pm
+++ b/lib/App/SD/ForeignReplica.pm
@@ -61,7 +61,8 @@ sub integrate_changeset {
     );
 
     my $changeset = $args{'changeset'};
-    return if $self->last_changeset_from_source( $changeset->original_source_uuid) >= $changeset->original_sequence_no;
+    return if $self->last_changeset_from_source(
+        $changeset->original_source_uuid) >= $changeset->original_sequence_no;
     $self->SUPER::integrate_changeset(%args);
 }
 
@@ -83,7 +84,7 @@ sub integrate_change {
     my ( $change, $changeset ) = validate_pos(
         @_,
         { isa => 'Prophet::Change' },
-        { isa => 'Prophet::ChangeSet' }
+        { isa => 'Prophet::ChangeSet' },
     );
 
     # don't push internal records
@@ -97,7 +98,10 @@ sub integrate_change {
 
 =head2 record_pushed_transactions
 
-Walk the set of transactions on the ticket whose id you've passed in, looking for updates by the 'current user' which happened after start_time and before now. Then mark those transactions as ones that originated in SD, so we don't accidentally push them later.
+Walk the set of transactions on the ticket whose id you've passed in, looking
+for updates by the 'current user' which happened after start_time and before
+now. Then mark those transactions as ones that originated in SD, so we don't
+accidentally push them later.
 
 =over
 
@@ -114,38 +118,38 @@ Walk the set of transactions on the ticket whose id you've passed in, looking fo
 sub record_pushed_transactions {
     my $self = shift;
     my %args = validate( @_,
-        { ticket => 1, changeset => { isa => 'Prophet::ChangeSet' }, start_time => 1} );
-
+        { ticket => 1,
+          changeset => { isa => 'Prophet::ChangeSet' }, start_time => 1} );
 
     my $earliest_valid_txn_date;
 
     # walk through every transaction on the ticket, starting with the latest
-    
+
     for my $txn ( $self->get_txn_list_by_date($args{ticket}) ) {
-       
         # walk backwards through all transactions on the ticket we just updated
-        # Skip any transaction where the remote user isn't me, this might include any transaction
-        # RT created with a scrip on your behalf
-   
+        # Skip any transaction where the remote user isn't me, this might
+        # include any transaction RT created with a scrip on your behalf
+
         next unless $txn->{creator} eq $self->foreign_username;
 
-        # get the completion time _after_ we do our next round trip to rt to try to make sure
-        # a bit of lag doesn't skew us to the wrong side of a 1s boundary
-      
-     
+        # get the completion time _after_ we do our next round trip to rt to
+        # try to make sure a bit of lag doesn't skew us to the wrong side of a
+        # 1s boundary
        if (!$earliest_valid_txn_date){
             my $change_window =  time() - $args{start_time};
-            # skip any transaction created more than 5 seconds before the push started.
-            # I can't think of any reason that number shouldn't be 1, but clocks are fickle
-            $earliest_valid_txn_date = $txn->{created} - ($change_window + 5); 
-        }      
+            # skip any transaction created more than 5 seconds before the push
+            # started. I can't think of any reason that number shouldn't be 1,
+            # but clocks are fickle
+            $earliest_valid_txn_date = $txn->{created} - ($change_window + 5);
+        }
 
         last if $txn->{created} < $earliest_valid_txn_date;
 
         # if the transaction id is older than the id of the last changeset
         # we got from the original source of this changeset, we're done
-        last if $txn->{id} <= $self->app_handle->handle->last_changeset_from_source($args{changeset}->original_source_uuid);
-        
+        last if $txn->{id} <= $self->app_handle->handle->last_changeset_from_source(
+           $args{changeset}->original_source_uuid);
+
         # if the transaction from RT is more recent than the most recent
         # transaction we got from the original source of the changeset
         # then we should record that we sent that transaction upstream
@@ -157,11 +161,11 @@ sub record_pushed_transactions {
         );
     }
 }
-    
+
 
 =head2 record_pushed_transaction $foreign_transaction_id, $changeset
 
-Record that this replica was the original source of $foreign_transaction_id 
+Record that this replica was the original source of $foreign_transaction_id
 (with changeset $changeset)
 
 =cut
@@ -169,10 +173,13 @@ Record that this replica was the original source of $foreign_transaction_id
 sub record_pushed_transaction {
     my $self = shift;
     my %args = validate( @_,
-        { transaction => 1, changeset => { isa => 'Prophet::ChangeSet' }, record => 1 } );
+        { transaction => 1, changeset => { isa => 'Prophet::ChangeSet' },
+          record => 1 } );
 
-    my $key =  join('-', "foreign-txn-from" , $self->uuid , 'record' , $args{record} , 'txn' , $args{transaction} );
-    my $value = join(':', $args{changeset}->original_source_uuid, $args{changeset}->original_sequence_no );
+    my $key = join('-', "foreign-txn-from" , $self->uuid ,
+                   'record' , $args{record} , 'txn' , $args{transaction} );
+    my $value = join(':', $args{changeset}->original_source_uuid,
+                     $args{changeset}->original_sequence_no );
 
     $self->store_local_metadata($key => $value);
 
@@ -193,7 +200,7 @@ once we've done a pull from the remote replica, we can safely expire
 all records of this type for the remote replica (they'll be
 obsolete)
 
-We use this cache to avoid integrating changesets we've pushed to the 
+We use this cache to avoid integrating changesets we've pushed to the
 remote replica when doing a subsequent pull
 
 =cut
@@ -225,52 +232,48 @@ sub traverse_changesets {
             );
 
             next unless $continue;
-
         }
 
-
-
         $args{callback}->(
             changeset                 => $changeset,
             after_integrate_changeset => sub {
                 $self->record_last_changeset_from_replica(
                     $changeset->original_source_uuid => $changeset->original_sequence_no );
 
-              # We're treating each individual ticket in the foreign system as its own 'replica'
-              # because of that, we need to hint to the push side of the system what the most recent
-              # txn on each ticket it has.
+                # We're treating each individual ticket in the foreign system
+                # as its own 'replica' because of that, we need to hint to the
+                # push side of the system what the most recent txn on each
+                # ticket it has.
                 my $previously_modified
-                    = App::SD::Util::string_to_datetime( $self->upstream_last_modified_date || '');
-                my $created_datetime = App::SD::Util::string_to_datetime( $changeset->created );
+                    = App::SD::Util::string_to_datetime(
+                       $self->upstream_last_modified_date || '');
+                my $created_datetime = App::SD::Util::string_to_datetime(
+                   $changeset->created );
                 $self->record_upstream_last_modified_date( $changeset->created )
                     if ( ( $created_datetime ? $created_datetime->epoch : 0 )
                     > ( $previously_modified ? $previously_modified->epoch : 0 ) );
-
             }
         );
         $args{reporting_callback}->($changeset) if ($args{reporting_callback});
-
     }
-
 }
 
 sub _construct_changeset_index_entry {
     my $self = shift;
     my $changeset = shift;
 
-    return [ $changeset->sequence_no, $changeset->original_source_uuid, $changeset->original_sequence_no, $changeset->calculate_sha1];
-
+    return [ $changeset->sequence_no, $changeset->original_source_uuid,
+             $changeset->original_sequence_no, $changeset->calculate_sha1 ];
 }
 
 sub remote_uri_path_for_id {
     die "your subclass needs to implement this to be able to ".
         "map a remote id to /ticket/id or soemthing";
-
 }
 
 =head2 uuid_for_remote_id $id
 
-lookup the uuid for the remote record id. If we don't find it, 
+lookup the uuid for the remote record id. If we don't find it,
 construct it out of the remote url and the remote uri path for the record id;
 
 =cut
@@ -286,7 +289,8 @@ sub _lookup_uuid_for_remote_id {
     my $self = shift;
     my ($id) = validate_pos( @_, 1 );
 
-    return $self->fetch_local_metadata('local_uuid_for_'.  $self->_url_based_uuid_for_remote_ticket_id( $id));
+    return $self->fetch_local_metadata(
+       'local_uuid_for_'.  $self->_url_based_uuid_for_remote_ticket_id($id));
 }
 
 sub _set_uuid_for_remote_id {
@@ -306,14 +310,12 @@ sub _url_based_uuid_for_remote_ticket_id {
         $self->remote_url
         . $self->remote_uri_path_for_id( $id )
     );
-
 }
 
-
-# This mapping table stores uuids for tickets we've synced from a remote database
-# Basically, if we created the ticket to begin with, then we'll know its uuid
-# if we pulled the ticket from the foreign replica then its uuid will be generated
-# based on a UUID-from-ticket-url scheme
+# This mapping table stores uuids for tickets we've synced from a remote
+# database Basically, if we created the ticket to begin with, then we'll know
+# its uuid if we pulled the ticket from the foreign replica then its uuid will
+# be generated based on a UUID-from-ticket-url scheme
 
 sub remote_id_for_uuid {
     my ( $self, $uuid_or_luid ) = @_;
@@ -351,7 +353,6 @@ sub _set_remote_id_for_uuid {
     );
     $ticket->load( uuid => $args{'uuid'} );
     $ticket->set_props( props => { $self->uuid.'-id' => $args{'remote_id'} } );
-
 }
 
 =head2 record_remote_id_for_pushed_record
diff --git a/lib/App/SD/Replica/trac.pm b/lib/App/SD/Replica/trac.pm
index 9a2b9af..14063b0 100644
--- a/lib/App/SD/Replica/trac.pm
+++ b/lib/App/SD/Replica/trac.pm
@@ -10,12 +10,10 @@ use constant scheme => 'trac';
 use constant pull_encoder => 'App::SD::Replica::trac::PullEncoder';
 use constant push_encoder => 'App::SD::Replica::trac::PushEncoder';
 
-
-use Prophet::ChangeSet;
-
 has trac => ( isa => 'Net::Trac::Connection', is => 'rw');
 has remote_url => ( isa => 'Str', is => 'rw');
 has query => ( isa => 'Maybe[Str]', is => 'rw');
+
 sub foreign_username { return shift->trac->user(@_) }
 
 sub BUILD {
@@ -57,16 +55,18 @@ sub BUILD {
     $self->trac->ensure_logged_in;
 }
 
-
-
 sub get_txn_list_by_date {
     my $self   = shift;
     my $ticket = shift;
 
     my $ticket_obj = Net::Trac::Ticket->new( connection => $self->trac);
     $ticket_obj->load($ticket);
-        
-    my @txns   = map { { id => $_->date->epoch, creator => $_->author, created => $_->date->epoch } } sort {$b->date <=> $a->date }  @{$ticket_obj->history->entries};
+
+    my @txns   = map {
+        { id => $_->date->epoch, creator => $_->author,
+          created => $_->date->epoch }
+    } sort {$b->date <=> $a->date }  @{$ticket_obj->history->entries};
+
     return @txns;
 }
 
@@ -119,7 +119,6 @@ sub database_settings {
     };
 }
 
-
 __PACKAGE__->meta->make_immutable;
 no Any::Moose;
 
diff --git a/lib/App/SD/Replica/trac/PullEncoder.pm b/lib/App/SD/Replica/trac/PullEncoder.pm
index 5d2aed8..c0fcb70 100644
--- a/lib/App/SD/Replica/trac/PullEncoder.pm
+++ b/lib/App/SD/Replica/trac/PullEncoder.pm
@@ -5,6 +5,9 @@ extends 'App::SD::ForeignReplica::PullEncoder';
 use Params::Validate qw(:all);
 use Memoize;
 
+use Prophet::ChangeSet;
+use Prophet::Change;
+
 has sync_source => (
     isa => 'App::SD::Replica::trac',
     is  => 'rw',
@@ -26,7 +29,7 @@ sub ticket_last_modified {
 sub translate_ticket_state {
     my $self          = shift;
     my $ticket_object = shift;
-    my $transactions = shift;    
+    my $transactions = shift;
     my $content = $ticket_object->description;
     my $ticket_data = {
 
@@ -47,9 +50,6 @@ sub translate_ticket_state {
         cc          => ( $ticket_object->cc || undef ),
     };
 
-
-
-
     # delete undefined and empty fields
     delete $ticket_data->{$_}
         for grep !defined $ticket_data->{$_} || $ticket_data->{$_} eq '', keys %$ticket_data;
@@ -66,27 +66,29 @@ Returns a array of all tickets found matching your QUERY hash.
 sub find_matching_tickets {
     my $self  = shift;
     my %query = (@_);
-   my $last_changeset_seen_dt =   $self->_only_pull_tickets_modified_after();
+    my $last_changeset_seen_dt =   $self->_only_pull_tickets_modified_after();
     $self->sync_source->log("Searching for tickets");
 
-    my $search = Net::Trac::TicketSearch->new( connection => $self->sync_source->trac, limit => 9999 );
+    my $search = Net::Trac::TicketSearch->new(
+        connection => $self->sync_source->trac, limit => 9999 );
     $search->query(%query);
     my @results = @{$search->results};
      $self->sync_source->log("Trimming things after our last pull");
     if ($last_changeset_seen_dt) {
         # >= is wasteful but may catch race conditions
-        @results = grep {$_->last_modified >= $last_changeset_seen_dt} @results; 
+        @results = grep {$_->last_modified >= $last_changeset_seen_dt} @results;
     }
     return \@results;
 }
 
 =head2 find_matching_transactions { ticket => $id, starting_transaction => $num  }
 
-Returns a reference to an array of all transactions (as hashes) on ticket $id after transaction $num.
+Returns a reference to an array of all transactions (as hashes) on ticket $id
+after transaction $num.
 
 =cut
 
-sub find_matching_transactions { 
+sub find_matching_transactions {
     my $self = shift;
     my %args = validate( @_, { ticket => 1, starting_transaction => 1 } );
     my @raw_txns = @{$args{ticket}->history->entries};
@@ -139,10 +141,10 @@ sub transcode_create_txn {
     my $create_data = shift;
     my $final_data = shift;
     my $ticket      = $txn->ticket;
-             # this sequence_no only works because trac tickets only allow one update 
-             # per ticket per second.
-             # we decrement by 1 on the off chance that someone created and 
-             # updated the ticket in the first second
+    # this sequence_no only works because trac tickets only allow one update
+    # per ticket per second.
+    # we decrement by 1 on the off chance that someone created and
+    # updated the ticket in the first second
     my $changeset = Prophet::ChangeSet->new(
         {   original_source_uuid => $self->sync_source->uuid_for_remote_id( $ticket->id ),
             original_sequence_no => ( $ticket->created->epoch-1),
diff --git a/lib/App/SD/Replica/trac/PushEncoder.pm b/lib/App/SD/Replica/trac/PushEncoder.pm
index b8d2a9b..9bb5187 100644
--- a/lib/App/SD/Replica/trac/PushEncoder.pm
+++ b/lib/App/SD/Replica/trac/PushEncoder.pm
@@ -1,16 +1,16 @@
 package App::SD::Replica::trac::PushEncoder;
-use Any::Moose; 
+use Any::Moose;
 use Params::Validate;
 use Time::HiRes qw/usleep/;
-has sync_source => 
+
+has sync_source =>
     ( isa => 'App::SD::Replica::trac',
       is => 'rw');
 
 extends 'App::SD::ForeignReplica::PushEncoder';
 
-
 sub after_integrate_change {
-  usleep(1100); # trac only accepts one ticket update per second. Yes. 
+    usleep(1100); # trac only accepts one ticket update per second. Yes.
 }
 
 sub integrate_ticket_update {
@@ -25,8 +25,10 @@ sub integrate_ticket_update {
     my $remote_ticket_id =
       $self->sync_source->remote_id_for_uuid( $change->record_uuid );
     my $ticket = Net::Trac::Ticket->new( connection => $self->sync_source->trac);
-    $ticket->load($remote_ticket_id);
-    $ticket->update( %{ $self->_recode_props_for_integrate($change) } );
+    $ticket->load($remote_ticket_id) or
+        die "couldn't load remote track ticket $remote_ticket_id\n";
+    $ticket->update( %{ $self->_recode_props_for_integrate($change) } ) or
+        die "couldn't update remote track ticket $remote_ticket_id\n";
     return $remote_ticket_id;
 }
 
@@ -48,7 +50,8 @@ sub integrate_ticket_create {
 
 sub integrate_comment {
     my $self = shift;
-    my ($change, $changeset) = validate_pos( @_, { isa => 'Prophet::Change' }, {isa => 'Prophet::ChangeSet'} );
+    my ($change, $changeset) = validate_pos( @_,
+        { isa => 'Prophet::Change' }, {isa => 'Prophet::ChangeSet'} );
 
     # Figure out the remote site's ticket ID for this change's record
 
@@ -59,11 +62,13 @@ sub integrate_comment {
     $ticket->load($ticket_id);
     $ticket->comment( $props{content});
     return $ticket_id;
-} 
+}
 
 sub integrate_attachment {
-    my ($self, $change, $changeset ) = validate_pos( @_, { isa => 'App::SD::Replica::trac::PushEncoder'}, { isa => 'Prophet::Change' }, { isa => 'Prophet::ChangeSet' });
-
+    my ($self, $change, $changeset ) = validate_pos( @_,
+      { isa => 'App::SD::Replica::trac::PushEncoder'},
+      { isa => 'Prophet::Change' },
+      { isa => 'Prophet::ChangeSet' });
 
     my %props = map { $_->name => $_->new_value } $change->prop_changes;
 

commit abeed5523c2099dca690d6bdf1f857e367fd6d17
Author: Christine Spang <christine at debian.org>
Date:   Sun Feb 27 10:04:50 2011 -0500

    abstract out login_loop() and use in RT sync too

diff --git a/lib/App/SD/ForeignReplica.pm b/lib/App/SD/ForeignReplica.pm
index 2d8fba1..98f84a2 100644
--- a/lib/App/SD/ForeignReplica.pm
+++ b/lib/App/SD/ForeignReplica.pm
@@ -2,6 +2,8 @@ package App::SD::ForeignReplica;
 use Any::Moose;
 use Params::Validate qw/:all/;
 
+use Try::Tiny;
+
 extends 'Prophet::ForeignReplica';
 
 
@@ -385,6 +387,67 @@ sub upstream_last_modified_date {
     return $self->fetch_local_metadata('last_modified_date');
 }
 
+=head2 login_loop
+
+Loop on prompting for username/password until login is successful; user can
+abort with ^C.
+
+Saves username and password to the replica's configuration file
+upon successful login.
+
+params:
+- uri             # login url
+- username_prompt # optional; custom username prompt
+- secret_prompt   # optional; custom secret prompt
+- login_callback  # coderef of code that attempts login; should throw exception
+                  # on error
+- catch_callback  # optional; process thrown exception message (e.g. munge
+                  # in some way and then print to STDERR)
+
+returns: ($username, $password)
+
+=cut
+
+sub login_loop {
+    my $self = shift;
+    my %args = @_;
+
+    my $login_successful = 0;
+    my ($username, $password);
+
+    my %login_args = ( uri => $args{uri}, username => $username );
+    $login_args{username_prompt} = $args{username_prompt}
+        if $args{username_prompt};
+    $login_args{secret_prompt} = $args{secret_prompt}
+        if $args{secret_prompt};
+
+    while (!$login_successful) {
+        ( $username, $password ) = $self->prompt_for_login(%login_args);
+
+        $self->foreign_username($username) if ($username);
+
+        try {
+            $args{login_callback}->($self, $username, $password);
+            $login_successful = 1;
+        } catch {
+            if ($args{catch_callback}) {
+                $args{catch_callback}->($_);
+            }
+            else {
+                warn "\n$_\n\n";
+            }
+        };
+    }
+    # only save username/password if login was successful
+    $self->save_username_and_token( $username, $password );
+
+    return ($username, $password);
+}
+
+sub foreign_username {
+    die "replica class must implement foreign_username";
+}
+
 __PACKAGE__->meta->make_immutable;
 no Any::Moose;
 
diff --git a/lib/App/SD/Replica/hm.pm b/lib/App/SD/Replica/hm.pm
index 7cb707b..539b050 100644
--- a/lib/App/SD/Replica/hm.pm
+++ b/lib/App/SD/Replica/hm.pm
@@ -6,8 +6,8 @@ use URI;
 use Memoize;
 use Prophet::ChangeSet;
 use File::Temp 'tempdir';
-use Try::Tiny;
 use Carp;
+use Try::Tiny;
 
 has hm               => ( isa => 'Net::Jifty', is => 'rw' );
 has remote_url       => ( isa => 'Str',        is => 'rw' );
@@ -52,49 +52,41 @@ sub BUILD {
     }
     $self->remote_url("$uri");
 
-    my $login_successful = 0;
-
-    while (!$login_successful) {
-        ( $username, $password )
-            = $self->prompt_for_login(
-                uri      => $uri,
-                username => $username,
-                # remind the user that hiveminder logins are email addresses
-                username_prompt => sub {
-                    my $uri = shift;
-                    return "Login email for $uri: ";
-                },
-            ) unless $password;
-
-        $self->foreign_username($username) if ($username);
-
-        try {
-            $self->hm(
-                Net::Jifty->new(
-                    site        => $self->remote_url,
-                    cookie_name => 'JIFTY_SID_HIVEMINDER',
-
-                    email    => $username,
-                    password => $password
-                )
-            );
-            $login_successful = 1;
-        } catch {
-            # Net::Jifty uses Carp::confess to deal with login problems :(
-            ($username, $password) = (undef, undef);
-            my $error_message = (split /\n/, $_)[0];
-            $error_message =~ s/ at .* line [0-9]+$//;
-            warn "\n$error_message\n\n";
-        };
+    unless ( $password ) {
+        ($username, $password) = $self->login_loop(
+            uri      => $uri,
+            username => $username,
+            # remind the user that hiveminder logins are email addresses
+            username_prompt => sub {
+                my $uri = shift;
+                return "Login email for $uri: ";
+            },
+            login_callback => sub {
+                my ($self, $username, $password) = @_;
+
+                $self->hm(
+                    Net::Jifty->new(
+                        site        => $self->remote_url,
+                        cookie_name => 'JIFTY_SID_HIVEMINDER',
+                        email    => $username,
+                        password => $password
+                    )
+                );
+            },
+            catch_callback => sub {
+                my $verbose_error = shift;
+                # Net::Jifty uses Carp::confess to deal with login problems :(
+                my $error_message = (split /\n/, $verbose_error)[0];
+                $error_message =~ s/ at .* line [0-9]+$//;
+                warn "\n$error_message\n\n";
+            }
+        );
     }
 
     if ($props) {
         my %props = split /=|;/, $props;
         $self->props( \%props );
     }
-
-    # only save username/password if login was successful
-    $self->save_username_and_token( $username, $password );
 }
 
 sub request_failed {
diff --git a/lib/App/SD/Replica/rt.pm b/lib/App/SD/Replica/rt.pm
index d811d8f..5032ea4 100644
--- a/lib/App/SD/Replica/rt.pm
+++ b/lib/App/SD/Replica/rt.pm
@@ -5,6 +5,7 @@ extends qw/App::SD::ForeignReplica/;
 use Params::Validate qw(:all);
 use File::Temp 'tempdir';
 use Memoize;
+use Try::Tiny;
 
 use constant scheme => 'rt';
 use constant pull_encoder => 'App::SD::Replica::rt::PullEncoder';
@@ -13,11 +14,11 @@ use constant push_encoder => 'App::SD::Replica::rt::PushEncoder';
 
 use Prophet::ChangeSet;
 
-has rt => ( isa => 'RT::Client::REST', is => 'rw');
-has remote_url => ( isa => 'Str', is => 'rw');
-has rt_queue => ( isa => 'Str', is => 'rw');
-has query => ( isa => 'Str', is => 'rw');
-has rt_username => (isa => 'Str', is => 'rw');
+has rt               => ( isa => 'RT::Client::REST', is => 'rw');
+has remote_url       => ( isa => 'Str', is              => 'rw');
+has rt_queue         => ( isa => 'Str', is              => 'rw');
+has query            => ( isa => 'Str', is              => 'rw');
+has foreign_username => ( isa => 'Str', is              => 'rw' );
 
 sub BUILD {
     my $self = shift;
@@ -47,25 +48,21 @@ sub BUILD {
     $self->query( ( $query ?  "($query) AND " :"") . " Queue = '$type'" );
     $self->rt( RT::Client::REST->new( server => $server ) );
 
-    ( $username, $password )
-        = $self->prompt_for_login(
+    unless ( $password ) {
+        ($username, $password) = $self->login_loop(
             uri      => $uri,
             username => $username,
-        ) unless $password;
+            login_callback => sub {
+                my ($self, $username, $password) = @_;
 
-    $self->rt_username($username);
-
-    eval {
-        $self->rt->login( username => $username, password => $password );
-    };
-    if ($@) {
-        die "Login to '$server' with username '$username' failed!\n"
-            ."Error was: $@.\n";
+                $self->rt->login( username => $username, password => $password );
+            },
+        );
     }
 }
 
-sub foreign_username { return shift->rt_username(@_)}
-  
+sub foreign_username { return shift->rt_username(@_) }
+
 sub get_txn_list_by_date {
     my $self   = shift;
     my $ticket = shift;

commit ba6f8b438b25f5f6452ef94542eb6ea25e5c9c4c
Author: Christine Spang <christine at debian.org>
Date:   Sun Feb 27 10:05:11 2011 -0500

    use Try::Tiny instead of eval

diff --git a/lib/App/SD/Replica/hm.pm b/lib/App/SD/Replica/hm.pm
index 539b050..f7e4c70 100644
--- a/lib/App/SD/Replica/hm.pm
+++ b/lib/App/SD/Replica/hm.pm
@@ -31,20 +31,22 @@ Open a connection to the source identified by C<$self->{url}>.
 
 sub BUILD {
     my $self = shift;
-    eval {
-        require Net::Jifty;
-    };
 
-    if ($@) {
+    # Require rather than use to defer load
+    try {
+        require Net::Jifty;
+    } catch {
         die "SD requires Net::Jifty to sync with a Hiveminder server.\n".
         "'cpan Net::Jifty' may sort this out for you";
-    }
+    };
 
     my ( $server, $props ) = $self->{url} =~ m/^hm:(.*?)(?:\|(.*))?$/
         or die
         "Can't parse Hiveminder server spec. Expected hm:http://hiveminder.com or hm:http://hiveminder.com|props";
+
     $self->url($server);
     my $uri = URI->new($server);
+
     my ( $username, $password );
     if ( $uri->can('userinfo') && ( my $auth = $uri->userinfo ) ) {
         ( $username, $password ) = split /:/, $auth, 2;
diff --git a/lib/App/SD/Replica/rt.pm b/lib/App/SD/Replica/rt.pm
index 5032ea4..c84daef 100644
--- a/lib/App/SD/Replica/rt.pm
+++ b/lib/App/SD/Replica/rt.pm
@@ -24,15 +24,14 @@ sub BUILD {
     my $self = shift;
 
     # Require rather than use to defer load
-    eval {
+    try {
         require RT::Client::REST;
         require RT::Client::REST::User;
         require RT::Client::REST::Ticket;
-    };
-    if ($@) {
-        warn $@ if $ENV{PROPHET_DEBUG};
+    } catch {
+        warn $_ if $ENV{PROPHET_DEBUG};
         die "RT::Client::REST is required to sync with RT foreign replicas.\n";
-    }
+    };
 
     my ( $server, $type, $query ) = $self->{url} =~ m{^rt:(https?://.*?)\|(.*?)\|(.*)$}
         or die "Can't parse RT server spec. Expected 'rt:http://example.com|QUEUE|QUERY'.\n"

commit 025987c44b483c6c979028ac146d36b6d81dbede
Author: Christine Spang <christine at debian.org>
Date:   Sun Feb 27 10:23:35 2011 -0500

    make trac sync use login_loop

diff --git a/lib/App/SD/Replica/trac.pm b/lib/App/SD/Replica/trac.pm
index 14063b0..6cbb101 100644
--- a/lib/App/SD/Replica/trac.pm
+++ b/lib/App/SD/Replica/trac.pm
@@ -5,28 +5,27 @@ extends qw/App::SD::ForeignReplica/;
 use Params::Validate qw(:all);
 use File::Temp 'tempdir';
 use Memoize;
+use Try::Tiny;
 
 use constant scheme => 'trac';
 use constant pull_encoder => 'App::SD::Replica::trac::PullEncoder';
 use constant push_encoder => 'App::SD::Replica::trac::PushEncoder';
 
-has trac => ( isa => 'Net::Trac::Connection', is => 'rw');
-has remote_url => ( isa => 'Str', is => 'rw');
-has query => ( isa => 'Maybe[Str]', is => 'rw');
-
-sub foreign_username { return shift->trac->user(@_) }
+has trac             => ( isa => 'Net::Trac::Connection', is => 'rw');
+has remote_url       => ( isa => 'Str', is                   => 'rw');
+has query            => ( isa => 'Maybe[Str]', is            => 'rw');
+has foreign_username => ( isa => 'Str', is                   => 'rw' );
 
 sub BUILD {
     my $self = shift;
 
     # Require rather than use to defer load
-    eval {
+    try {
         require Net::Trac;
-    };
-    if ($@) {
+    } catch {
         die "SD requires Net::Trac to sync with a Trac server.\n".
         "'cpan Net::Trac' may sort this out for you.\n";
-    }
+    };
 
     my ( $server, $type, $query ) = $self->{url} =~ m/^trac:(.*?)$/
         or die
@@ -39,20 +38,26 @@ sub BUILD {
     }
     $self->remote_url( $uri->as_string );
 
-    ( $username, $password )
-        = $self->prompt_for_login(
+    unless ( $password ) {
+        ($username, $password) = $self->login_loop(
             uri      => $uri,
             username => $username,
-        ) unless $password;
-
-    $self->trac(
-        Net::Trac::Connection->new(
-            url      => $self->remote_url,
-            user     => $username,
-            password => $password,
-        )
-    );
-    $self->trac->ensure_logged_in;
+            login_callback => sub {
+                my ($self, $username, $password) = @_;
+
+                $self->trac(
+                    Net::Trac::Connection->new(
+                        url      => $self->remote_url,
+                        user     => $username,
+                        password => $password,
+                    )
+                );
+                # Net::Trac doesn't give us enough information
+                # to give a better diagnostic message, unfortunately
+                die "login failed!\n" unless $self->trac->ensure_logged_in;
+            },
+        );
+    }
 }
 
 sub get_txn_list_by_date {

commit 39d5ee561f69dd5ee23d4a5b9de3be1bb4a40ff2
Author: Christine Spang <christine at debian.org>
Date:   Sun Feb 27 10:23:51 2011 -0500

    don't set foreign_username until successful login

diff --git a/lib/App/SD/ForeignReplica.pm b/lib/App/SD/ForeignReplica.pm
index 98f84a2..ab2adda 100644
--- a/lib/App/SD/ForeignReplica.pm
+++ b/lib/App/SD/ForeignReplica.pm
@@ -424,8 +424,6 @@ sub login_loop {
     while (!$login_successful) {
         ( $username, $password ) = $self->prompt_for_login(%login_args);
 
-        $self->foreign_username($username) if ($username);
-
         try {
             $args{login_callback}->($self, $username, $password);
             $login_successful = 1;
@@ -437,6 +435,7 @@ sub login_loop {
                 warn "\n$_\n\n";
             }
         };
+        $self->foreign_username($username) if ($username);
     }
     # only save username/password if login was successful
     $self->save_username_and_token( $username, $password );

commit d8b3d5482f915524517e02929f05a715c3f166fe
Author: Christine Spang <christine at debian.org>
Date:   Sun Feb 27 10:53:02 2011 -0500

    make gcode sync use login_loop() and make some other improvements

diff --git a/lib/App/SD/Replica/gcode.pm b/lib/App/SD/Replica/gcode.pm
index 5739ad9..5833a98 100644
--- a/lib/App/SD/Replica/gcode.pm
+++ b/lib/App/SD/Replica/gcode.pm
@@ -5,6 +5,7 @@ extends qw/App::SD::ForeignReplica/;
 use Params::Validate qw(:all);
 use File::Temp 'tempdir';
 use Memoize;
+use Try::Tiny;
 
 use constant scheme => 'gcode';
 use constant pull_encoder => 'App::SD::Replica::gcode::PullEncoder';
@@ -26,26 +27,24 @@ our %PROP_MAP = (
 );
 
 
-has query => ( isa => 'Str', is => 'rw');
-has gcode => ( isa => 'Net::Google::Code', is => 'rw');
-has project => ( isa => 'Str', is => 'rw');
+has query            => ( isa => 'Str', is               => 'rw');
+has gcode            => ( isa => 'Net::Google::Code', is => 'rw');
+has project          => ( isa => 'Str', is               => 'rw');
+has foreign_username => ( isa => 'Str', is               => 'rw' );
 
 sub remote_url { return "http://code.google.com/p/".shift->project}
-sub foreign_username { return shift->gcode->email(@_) }
 
 sub BUILD {
     my $self = shift;
+
     # Require rather than use to defer load
-    eval {
+    try {
         require Net::Google::Code;
         require Net::Google::Code::Issue;
-    };
-
-    if ($@) {
+    } catch {
         die "SD requires Net::Google::Code to sync with Google Code.\n".
         "'cpan Net::Google::Code' may sort this out for you.\n";
-    }
-
+    };
 
     $Net::Google::Code::Issue::USE_HYBRID = 1
       if $Net::Google::Code::VERSION ge '0.15';
@@ -56,25 +55,48 @@ sub BUILD {
 "Can't parse Google::Code server spec. Expected gcode:k9mail or gcode:user:password\@k9mail or gcode:user:password\@k9mail/q=string&can=all";
     $self->project($project);
     $self->query($query) if defined $query;
+
     my ( $email, $password );
-    # ask password only if there is userinfo but not password
-    if ( $userinfo ) {
-        $userinfo =~ s/\@$//;
-        ( $email, $password ) = split /:/, $userinfo;
-        ( undef, $password ) = $self->prompt_for_login( "gcode:$project", $email ) unless $password;
-        $self->gcode(
-            Net::Google::Code->new(
-                project  => $self->project,
-                email    => $email,
-                password => $password,
-            )
+    unless ( $password ) {
+        ($email, $password) = $self->login_loop(
+            uri      => $project,
+            username => $email,
+            # remind the user that gcode logins are email addresses
+            username_prompt => sub {
+                my $project = shift;
+                return "Login email for $project (blank OK for read-only): ";
+            },
+            secret_prompt => sub {
+                my ($uri, $username) = @_;
+                return "Password for $username (blank OK for read-only): ";
+            },
+            login_callback => sub {
+                my ($self, $email, $password) = @_;
+                my %gcode_args = ( project => $self->project );
+                $gcode_args{$email} = $email if $email;
+                $gcode_args{$password} = $password if $password;
+
+                $self->gcode( Net::Google::Code->new(%gcode_args) );
+            },
         );
     }
-    else {
-        $self->gcode( Net::Google::Code->new( project => $self->project ) );
-    }
 
-    $self->gcode->load();
+    # Net::Google::Code->new() will never fail, and if ->load() fails
+    # it generally means that the project name was wrong, since auth
+    # isn't performed on load. So since there's no point in re-prompting for
+    # the username / password, we move ->load() to a separate try/catch block
+    # outside of login_loop().
+    try {
+        $self->gcode->load();
+    } catch {
+        if ( $_ =~ m{Error GETing .*: Not Found} ) {
+            die "The Google Code project '$project' does not exist. Aborting!\n";
+        }
+        else {
+            # some other error
+            die $_;
+        }
+    }
 }
 
 sub get_txn_list_by_date {

commit 26ccda4ee5bc6039e8e98262c18a6906091f1009
Author: Christine Spang <christine at debian.org>
Date:   Sun Feb 27 10:53:42 2011 -0500

    allow prompting for just password if username already specified in login_loop

diff --git a/lib/App/SD/ForeignReplica.pm b/lib/App/SD/ForeignReplica.pm
index ab2adda..6beec78 100644
--- a/lib/App/SD/ForeignReplica.pm
+++ b/lib/App/SD/ForeignReplica.pm
@@ -420,6 +420,8 @@ sub login_loop {
         if $args{username_prompt};
     $login_args{secret_prompt} = $args{secret_prompt}
         if $args{secret_prompt};
+    # allow prompting for just password if username already specified
+    $login_args{username} = $args{username} if $args{username};
 
     while (!$login_successful) {
         ( $username, $password ) = $self->prompt_for_login(%login_args);

commit c7632007cf1a70357721b70a43528394ca88da94
Author: Christine Spang <christine at debian.org>
Date:   Sun Feb 27 14:01:14 2011 -0500

    don't print the return value of the progress bar sub
    
    The progress bar sub does the printing itself; we don't want to print '1'
    everywhere.

diff --git a/lib/App/SD/ForeignReplica/PullEncoder.pm b/lib/App/SD/ForeignReplica/PullEncoder.pm
index 88caf16..94bc0dc 100644
--- a/lib/App/SD/ForeignReplica/PullEncoder.pm
+++ b/lib/App/SD/ForeignReplica/PullEncoder.pm
@@ -30,7 +30,7 @@ sub run {
     for my $ticket (@$tickets) {
         $counter++;
         my $changesets;
-        print $progress->();
+        $progress->();
         $self->sync_source->log_debug( "Fetching $counter of " . scalar @$tickets  . " tickets");
         ( $last_modified, $changesets ) = $self->transcode_ticket( $ticket, $last_modified );
         unshift @changesets, @$changesets;

commit 7c19d50d8a845ddfb69feffd6b8fcebde3235bbe
Author: Christine Spang <christine at debian.org>
Date:   Sun Feb 27 21:06:33 2011 -0500

    pass password into prompt_for_login

diff --git a/lib/App/SD/ForeignReplica.pm b/lib/App/SD/ForeignReplica.pm
index 6beec78..78c34b3 100644
--- a/lib/App/SD/ForeignReplica.pm
+++ b/lib/App/SD/ForeignReplica.pm
@@ -397,6 +397,8 @@ upon successful login.
 
 params:
 - uri             # login url
+- username        # optional; a pre-seeded username
+- password        # optional; a pre-seeded password
 - username_prompt # optional; custom username prompt
 - secret_prompt   # optional; custom secret prompt
 - login_callback  # coderef of code that attempts login; should throw exception
@@ -421,7 +423,11 @@ sub login_loop {
     $login_args{secret_prompt} = $args{secret_prompt}
         if $args{secret_prompt};
     # allow prompting for just password if username already specified
+    # and vice-versa for password
+    # if both are specified, we still want to loop in case the
+    # credentials are wrong
     $login_args{username} = $args{username} if $args{username};
+    $login_args{password} = $args{password} if $args{password};
 
     while (!$login_successful) {
         ( $username, $password ) = $self->prompt_for_login(%login_args);

commit af61a2e2949c1276ffd08d838fad722ac1f9a7aa
Author: Christine Spang <christine at debian.org>
Date:   Sun Feb 27 21:07:10 2011 -0500

    use login_loop in github sync

diff --git a/lib/App/SD/Replica/github.pm b/lib/App/SD/Replica/github.pm
index 3e1dd7a..915742c 100644
--- a/lib/App/SD/Replica/github.pm
+++ b/lib/App/SD/Replica/github.pm
@@ -19,6 +19,7 @@ has remote_url => ( isa => 'Str',             is => 'rw' );
 has owner      => ( isa => 'Str',             is => 'rw' );
 has repo       => ( isa => 'Str',             is => 'rw' );
 has query      => ( isa => 'Str',             is => 'rw' );
+has foreign_username => ( isa => 'Str', is              => 'rw' );
 
 our %PROP_MAP = ( state => 'status', title => 'summary' );
 
@@ -53,35 +54,39 @@ sub BUILD {
         $uri = 'http://github.com/';
     }
 
+    # try loading github username & token from git configuration
     # see http://github.com/blog/180-local-github-config
-    if ( !$api_token ) {
+    unless ( $api_token ) {
         my $config = Config::GitLike::Git->new;
         $config->load;
         $username  = $config->get(key => 'github.user');
         $api_token = $config->get(key => 'github.token');
     }
 
-    ( $username, $api_token )
-        = $self->prompt_for_login(
-            uri => $uri,
-            username => $username,
-            secret_prompt => sub {
-                my ($uri, $username) = @_;
-                return "GitHub API token for $username (from ${uri}account): ";
-            },
-        ) unless $api_token;
+    ($username, $api_token) = $self->login_loop(
+        uri      => $uri,
+        username => $username,
+        password => $api_token,
+        secret_prompt => sub {
+            my ($uri, $username) = @_;
+            return "GitHub API token for $username (from ${uri}account): ";
+        },
+        login_callback => sub {
+            my ($self, $username, $api_token) = @_;
+
+            $self->github(
+                Net::GitHub->new(
+                    login => $username,
+                    token => $api_token,
+                    repo  => $repo,
+                    owner => $owner,
+                ) );
+        },
+    );
 
     $self->remote_url("$uri");
     $self->owner( $owner );
     $self->repo( $repo );
-
-    $self->github(
-        Net::GitHub->new(
-            login => $username,
-            token => $api_token,
-            repo  => $repo,
-            owner => $owner,
-        ) );
 }
 
 sub record_pushed_transactions {}

commit a0ee3de1cb6cbe63028d71f8b4c6642006175034
Author: Christine Spang <christine at debian.org>
Date:   Sun Feb 27 21:07:25 2011 -0500

    formatting cleanup in github sync

diff --git a/lib/App/SD/Replica/github.pm b/lib/App/SD/Replica/github.pm
index 915742c..dd42c4a 100644
--- a/lib/App/SD/Replica/github.pm
+++ b/lib/App/SD/Replica/github.pm
@@ -112,8 +112,8 @@ sub remote_uri_path_for_id {
 sub database_settings {
     my $self = shift;
     return {
-# TODO limit statuses too? the problem is github's statuses are so poor,
-# it only has 2 statuses: 'open' and 'closed'.
+    # TODO limit statuses too? the problem is github's statuses are so poor,
+    # it only has 2 statuses: 'open' and 'closed'.
         project_name => $self->owner . '/' . $self->repo,
     };
 
diff --git a/lib/App/SD/Replica/github/PullEncoder.pm b/lib/App/SD/Replica/github/PullEncoder.pm
index b10bae5..e121325 100644
--- a/lib/App/SD/Replica/github/PullEncoder.pm
+++ b/lib/App/SD/Replica/github/PullEncoder.pm
@@ -66,7 +66,8 @@ sub _only_pull_tickets_modified_after {
 
 =head2 find_matching_transactions { ticket => $id, starting_transaction => $num  }
 
-Returns a reference to an array of all transactions (as hashes) on ticket $id after transaction $num.
+Returns a reference to an array of all transactions (as hashes) on ticket $id
+after transaction $num.
 
 =cut
 

-----------------------------------------------------------------------



More information about the Bps-public-commit mailing list