[Bps-public-commit] SD - A distributed issue tracker branch, master, updated. ba375d4f466053e4aae8e83e7f118244b8dbfd4a
jesse
jesse at bestpractical.com
Mon Feb 16 18:19:43 EST 2009
The branch, master has been updated
via ba375d4f466053e4aae8e83e7f118244b8dbfd4a (commit)
from 75768136db2286d854a14694024a7bef8afdd21c (commit)
Summary of changes:
inc/Module/Install/Share.pm | 62 +------------------------------
lib/App/SD/Replica/rt.pm | 66 +++++++++++++++++++++++++++++----
lib/App/SD/Replica/rt/PullEncoder.pm | 35 +++---------------
lib/App/SD/Replica/rt/PushEncoder.pm | 4 ++
t/sd-rt/bogus-rt-data.t | 3 --
t/sd-rt/race-condition.t | 21 +++--------
t/sd-rt/sd-rt-permission.t | 3 --
7 files changed, 75 insertions(+), 119 deletions(-)
- Log -----------------------------------------------------------------
commit ba375d4f466053e4aae8e83e7f118244b8dbfd4a
Author: Jesse Vincent <jesse at bestpractical.com>
Date: Mon Feb 16 18:18:53 2009 -0500
RT replica: Add a rolling window for sorting out which changes you've pushed
upstream; Better track previously pulled changes and changes you
didn't originate when pushing
diff --git a/inc/Module/Install/Share.pm b/inc/Module/Install/Share.pm
index b32ad2c..c3e732e 100644
--- a/inc/Module/Install/Share.pm
+++ b/inc/Module/Install/Share.pm
@@ -1,3 +1,4 @@
+#line 1
package Module::Install::Share;
use strict;
@@ -63,63 +64,4 @@ END_MAKEFILE
__END__
-=pod
-
-=head1 NAME
-
-Module::Install::Share - Install non-code files for use during run-time
-
-=head1 SYNOPSIS
-
- # Put everything inside ./share/ into the distribution 'auto' path
- install_share 'share';
-
- # Same thing as above using the default directory name
- install_share;
-
-=head1 DESCRIPTION
-
-As well as Perl modules and Perl binary applications, some distributions
-need to install read-only data files to a location on the file system
-for use at run-time.
-
-XML Schemas, L<YAML> data files, and L<SQLite> databases are examples of
-the sort of things distributions might typically need to have available
-after installation.
-
-C<Module::Install::Share> is a L<Module::Install> extension that provides
-commands to allow these files to be installed to the applicable location
-on disk.
-
-To locate the files after installation so they can be used inside your
-module, see this extension's companion module L<File::ShareDir>.
-
-=head1 TO DO
-
-Currently C<install_share> installs not only the files you want, but
-if called by the author will also copy F<.svn> and other source-control
-directories, and other junk.
-
-Enhance this to copy only files under F<share> that are in the
-F<MANIFEST>, or possibly those not in F<MANIFEST.SKIP>.
-
-=head1 AUTHORS
-
-Audrey Tang E<lt>autrijus at autrijus.orgE<gt>
-
-Adam Kennedy E<lt>adamk at cpan.orgE<gt>
-
-=head1 SEE ALSO
-
-L<Module::Install>, L<File::ShareDir>
-
-=head1 COPYRIGHT
-
-Copyright 2006 Audrey Tang, Adam Kennedy.
-
-This program is free software; you can redistribute it and/or modify it
-under the same terms as Perl itself.
-
-See L<http://www.perl.com/perl/misc/Artistic.html>
-
-=cut
+#line 125
diff --git a/lib/App/SD/Replica/rt.pm b/lib/App/SD/Replica/rt.pm
index d7c985c..a654e37 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 Path::Class;
use File::Temp 'tempdir';
+use HTTP::Date;
use Memoize;
use constant scheme => 'rt';
@@ -18,6 +19,7 @@ has rt => ( isa => 'RT::Client::REST', is => 'rw');
has remote_url => ( isa => 'Str', is => 'rw');
has rt_queue => ( isa => 'Str', is => 'rw');
has rt_query => ( isa => 'Str', is => 'rw');
+has rt_username => (isa => 'Str', is => 'rw');
sub BUILD {
my $self = shift;
@@ -40,8 +42,9 @@ sub BUILD {
$self->rt_query( ( $query ? "($query) AND " :"") . " Queue = '$type'" );
$self->rt( RT::Client::REST->new( server => $server ) );
- ( $username, $password ) = $self->prompt_for_login( $uri, $username )
- unless $password;
+ ( $username, $password ) = $self->prompt_for_login( $uri, $username ) unless $password;
+
+ $self->rt_username($username);
$self->rt->login( username => $username, password => $password );
}
@@ -49,23 +52,46 @@ sub BUILD {
sub record_pushed_transactions {
my $self = shift;
my %args = validate( @_,
- { ticket => 1, changeset => { isa => 'Prophet::ChangeSet' } } );
+ { 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 ( reverse RT::Client::REST::Ticket->new(
+ for my $txn ( sort {$b->{'Created'} <=> $a->{'Created'}} RT::Client::REST::Ticket->new(
rt => $self->rt,
id => $args{'ticket'})->transactions->get_iterator->()) {
+
+ # 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
+ next unless $txn->creator eq $self->rt_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
+ my $txn_created = HTTP::Date::str2time($txn->created);
+ 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);
+ }
+
+ 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->last_changeset_from_source(
- $args{changeset}->original_source_uuid
- );
+ last if $txn->id <= $self->upstream_last_txn();
+
# 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
- # XXX TODO - THIS IS WRONG - we should only be recording transactions we pushed
$self->record_pushed_transaction(
transaction => $txn->id,
changeset => $args{'changeset'},
@@ -75,6 +101,30 @@ sub record_pushed_transactions {
}
+sub record_upstream_last_modified_date {
+ my $self = shift;
+ my $date = shift;
+ return $self->store_local_metadata('last_modified_date' => $date);
+}
+
+sub upstream_last_modified_date {
+ my $self = shift;
+ return $self->fetch_local_metadata('last_modified_date');
+}
+
+sub upstream_last_txn {
+ my $self = shift;
+ return $self->fetch_local_metadata('last_txn_id');
+}
+
+sub record_upstream_last_txn {
+ my $self = shift;
+ my $id = shift;
+ return $self->store_local_metadata('last_txn_id' => $id);
+}
+
+
+
=head2 uuid
Return the replica's UUID
diff --git a/lib/App/SD/Replica/rt/PullEncoder.pm b/lib/App/SD/Replica/rt/PullEncoder.pm
index ee9acea..6304186 100644
--- a/lib/App/SD/Replica/rt/PullEncoder.pm
+++ b/lib/App/SD/Replica/rt/PullEncoder.pm
@@ -78,35 +78,10 @@ sub run {
$args{callback}->($_)
}
- $self->_record_upstream_last_modified_date($last_modified) if ($last_modified > $self->_upstream_last_modified_date);
- $self->_record_upstream_last_txn($last_txn) if ($last_txn > $self->_upstream_last_txn);
+ $self->sync_source->record_upstream_last_modified_date($last_modified) if ($last_modified > $self->sync_source->upstream_last_modified_date);
+ $self->sync_source->record_upstream_last_txn($last_txn) if ($last_txn > $self->sync_source->upstream_last_txn);
}
-sub _record_upstream_last_modified_date {
- my $self = shift;
- my $date = shift;
- return $self->sync_source->store_local_metadata('last_modified_date' => $date);
-}
-
-sub _upstream_last_modified_date {
- my $self = shift;
- return $self->sync_source->fetch_local_metadata('last_modified_date');
-}
-
-sub _upstream_last_txn {
- my $self = shift;
- return $self->sync_source->fetch_local_metadata('last_txn_id');
-}
-
-sub _record_upstream_last_txn {
- my $self = shift;
- my $id = shift;
- warn "Id is $id";
- return $self->sync_source->store_local_metadata('last_txn_id' => $id);
-}
-
-
-
sub _translate_final_ticket_state {
my $self = shift;
@@ -162,13 +137,13 @@ sub find_matching_tickets {
my ($query) = validate_pos(@_, 1);
# If we've ever synced, we can limit our search to only newer things
- if ($self->_upstream_last_modified_date){
+ if ($self->sync_source->upstream_last_modified_date){
# last modified date is in GMT and searches are in user-time XXX -check assumption
# because of this, we really want to back that date down by one day to catch overlap
# XXX TODO we are playing FAST AND LOOSE WITH DATE MATH
# XXX TODO THIS WILL HURT US SOME DAY
# At that time, Jesse will buy you a beer.
- my $before = HTTP::Date::str2time($self->_upstream_last_modified_date() ) - (86400 + 7200) ; # 26 hours ago deals with most any possible edge case
+ my $before = HTTP::Date::str2time($self->sync_source->upstream_last_modified_date() ) - (86400 + 7200) ; # 26 hours ago deals with most any possible edge case
$query = "($query) AND LastUpdated >= '".HTTP::Date::time2iso($before) ."'";
@@ -192,7 +167,7 @@ sub find_matching_transactions {
my $rt_handle = $self->sync_source->rt;
- my $latest = $self->_upstream_last_txn();
+ my $latest = $self->sync_source->upstream_last_txn();
for my $txn ( sort $rt_handle->get_transaction_ids( parent_id => $args{'ticket'} ) ) {
# Skip things calling code told us to skip
next if $txn < $args{'starting_transaction'};
diff --git a/lib/App/SD/Replica/rt/PushEncoder.pm b/lib/App/SD/Replica/rt/PushEncoder.pm
index c0111e1..15649d2 100644
--- a/lib/App/SD/Replica/rt/PushEncoder.pm
+++ b/lib/App/SD/Replica/rt/PushEncoder.pm
@@ -16,6 +16,9 @@ sub integrate_change {
);
my $id;
local $@;
+
+ my $before_integration = time();
+
eval {
if ( $change->record_type eq 'ticket'
and $change->change_type eq 'add_file' )
@@ -50,6 +53,7 @@ sub integrate_change {
$self->sync_source->record_pushed_transactions(
ticket => $id,
+ start_time => $before_integration,
changeset => $changeset
);
diff --git a/t/sd-rt/bogus-rt-data.t b/t/sd-rt/bogus-rt-data.t
index 568f487..ac7763b 100644
--- a/t/sd-rt/bogus-rt-data.t
+++ b/t/sd-rt/bogus-rt-data.t
@@ -91,8 +91,6 @@ run_output_matches_unordered(
$RT::Handle->dbh->do("UPDATE Tickets SET Status = 'rejected' WHERE id = ".$ticket->id);
RT::Client::REST::Ticket->new( rt => $rt, id => $ticket->id, status => 'open',)->store();
-TODO: {
- local $TODO = "need to deal with pulling from a foreign replica";
( $ret, $out, $err ) = run_script( 'sd', [ 'pull', '--from', $sd_rt_url, '--prefer', 'from' ] );
ok( $ret, $out );
run_output_matches_unordered(
@@ -100,6 +98,5 @@ run_output_matches_unordered(
[ 'ticket', 'list', '--regex', '.' ],
[ "$flyman_id Fly Man open", ]
);
-}
1;
diff --git a/t/sd-rt/race-condition.t b/t/sd-rt/race-condition.t
index d159e6b..aaffa4b 100644
--- a/t/sd-rt/race-condition.t
+++ b/t/sd-rt/race-condition.t
@@ -78,11 +78,7 @@ my ( $yatta_id, $flyman_id );
# make sure ticket is new
-run_output_matches(
- 'sd',
- [ 'ticket', 'list', '--regex', '.' ],
- [qr/(.*?)(?{ $flyman_id = $1 }) Fly Man new/]
-);
+run_output_matches( 'sd', [ 'ticket', 'list', '--regex', '.' ], [qr/(.*?)(?{ $flyman_id = $1 }) Fly Man new/]);
@@ -90,9 +86,7 @@ run_output_matches(
# comment on ticket in sd
-( $ret, $out, $err )
- = run_script( 'sd',
- [ 'ticket', 'comment', $flyman_id, '--content', 'helium is a noble gas' ] );
+( $ret, $out, $err ) = run_script( 'sd', [ 'ticket', 'comment', $flyman_id, '--content', 'helium is a noble gas' ] );
ok( $ret, $out );
like( $out, qr/Created comment/ );
@@ -102,22 +96,19 @@ like( $out, qr/Created comment/ );
# make sure ticket is new
-run_output_matches(
- 'sd',
- [ 'ticket', 'list', '--regex', '.' ],
- ["$flyman_id Fly Man new"]
-);
-
+run_output_matches( 'sd', [ 'ticket', 'list', '--regex', '.' ], ["$flyman_id Fly Man new"]);
+diag("About to push to RT");
# push to rt
{
my ( $ret, $out, $err ) = run_script( 'sd', [ 'push', '--to', $sd_rt_url ] );
- diag($out);
diag($err);
}
+
+
# make sure ticket is open in rt, since after we commented the scrip popped it open.
{
my $fetched_ticket = RT::Client::REST::Ticket->new(
diff --git a/t/sd-rt/sd-rt-permission.t b/t/sd-rt/sd-rt-permission.t
index 8a86d53..10f6ca8 100644
--- a/t/sd-rt/sd-rt-permission.t
+++ b/t/sd-rt/sd-rt-permission.t
@@ -69,10 +69,7 @@ as_alice {
ok($ret);
like($out, qr/No new changesets/);
- TODO: {
- local $TODO = "not coming through for some reason";
like($err, qr/No tickets found/);
- }
};
diag("grant read rights, ensure we can pull it");
-----------------------------------------------------------------------
More information about the Bps-public-commit
mailing list