[Rt-commit] rt branch, 4.4/external-storage, updated. rt-4.2.11-78-gfccaa3f

Shawn Moore shawn at bestpractical.com
Wed May 27 14:19:05 EDT 2015


The branch, 4.4/external-storage has been updated
       via  fccaa3f3b775779c9864535b664ff86bd49f0b16 (commit)
       via  ef18fcbba21d075f9f3c45074bf02007c2e4e6ad (commit)
       via  d5d2770c1d1f1786642ff61dd66f4d1c6886293e (commit)
       via  9f3ce5f2a332e84db86be1ac4583887fc015da61 (commit)
       via  9288f351243293571f8b887bf462f18a3b46d24d (commit)
       via  d90eb37c481c2222fee655fbb6274b0010023022 (commit)
       via  2ec02cade0047409a5345a3824fb28dead21a859 (commit)
       via  f01f43ec5ddcbbe998cd8b31ca8c3bf547db4e9d (commit)
      from  da32d146747ed1c6a16b05d4fc958cc684888a1f (commit)

Summary of changes:
 lib/RT/Attachment.pm               | 22 +++++----
 lib/RT/ExternalStorage/AmazonS3.pm | 82 +++++++++++++++++++++++++++++++-
 lib/RT/ExternalStorage/Disk.pm     |  2 +-
 lib/RT/ExternalStorage/Dropbox.pm  |  6 +--
 lib/RT/ObjectCustomFieldValue.pm   | 10 ++--
 sbin/rt-externalize-attachments.in | 96 +++++++++++++++++++++++++-------------
 t/externalstorage/basic.t          | 15 ++++--
 7 files changed, 179 insertions(+), 54 deletions(-)

- Log -----------------------------------------------------------------
commit f01f43ec5ddcbbe998cd8b31ca8c3bf547db4e9d
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Wed May 27 17:17:03 2015 +0000

    Copyediting for ExternalStorage docs

diff --git a/lib/RT/ExternalStorage/Disk.pm b/lib/RT/ExternalStorage/Disk.pm
index d8bc6af..63991bf 100644
--- a/lib/RT/ExternalStorage/Disk.pm
+++ b/lib/RT/ExternalStorage/Disk.pm
@@ -145,7 +145,7 @@ uncompressed.  The files are de-duplicated when they are saved; as such,
 if the same file appears in multiple transactions, only one copy will be
 stored on disk.
 
-The C<Path> must be readable by the webserver, and writable by the
+The C<Path> must be readable by the web server, and writable by the
 C<sbin/rt-externalize-attachments> script.  Because the majority of the
 attachments are in the filesystem, a simple database backup is thus
 incomplete.  It is B<extremely important> that I<backups include the
diff --git a/lib/RT/ExternalStorage/Dropbox.pm b/lib/RT/ExternalStorage/Dropbox.pm
index f97eaf0..0aa6e52 100644
--- a/lib/RT/ExternalStorage/Dropbox.pm
+++ b/lib/RT/ExternalStorage/Dropbox.pm
@@ -138,7 +138,7 @@ RT::ExternalStorage::Dropbox - Store files in the Dropbox cloud
 This storage option places attachments in the Dropbox shared file
 service.  The files are de-duplicated when they are saved; as such, if
 the same file appears in multiple transactions, only one copy will be
-stored on in Dropbox.
+stored in Dropbox.
 
 Files in Dropbox C<must not be modified or removed>; doing so may cause
 internal inconsistency.  It is also important to ensure that the Dropbox
@@ -147,7 +147,7 @@ its space usage.
 
 =head1 SETUP
 
-In order to use this stoage type, a new application must be registered
+In order to use this storage type, a new application must be registered
 with Dropbox:
 
 =over

commit 2ec02cade0047409a5345a3824fb28dead21a859
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Wed May 27 17:46:01 2015 +0000

    Log when we create a new bucket

diff --git a/lib/RT/ExternalStorage/AmazonS3.pm b/lib/RT/ExternalStorage/AmazonS3.pm
index 6b4440f..d909585 100644
--- a/lib/RT/ExternalStorage/AmazonS3.pm
+++ b/lib/RT/ExternalStorage/AmazonS3.pm
@@ -114,7 +114,10 @@ sub Init {
             bucket    => $self->Bucket,
             acl_short => 'private',
         } );
-        unless ($ok) {
+        if ($ok) {
+            RT->Logger->debug("Created new bucket '".$self->Bucket."' on AmazonS3");
+        }
+        else {
             RT->Logger->error("Can't create new bucket '".$self->Bucket."' on AmazonS3: ".$S3->errstr);
             return;
         }

commit d90eb37c481c2222fee655fbb6274b0010023022
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Wed May 27 17:47:29 2015 +0000

    S3 setup instructions

diff --git a/lib/RT/ExternalStorage/AmazonS3.pm b/lib/RT/ExternalStorage/AmazonS3.pm
index d909585..4be9d71 100644
--- a/lib/RT/ExternalStorage/AmazonS3.pm
+++ b/lib/RT/ExternalStorage/AmazonS3.pm
@@ -154,6 +154,83 @@ sub DownloadURLFor {
     return;
 }
 
+=head1 NAME
+
+RT::ExternalStorage::AmazonS3 - Store files in Amazon's S3 cloud
+
+=head1 SYNOPSIS
+
+    Set(%ExternalStorage,
+        Type            => 'AmazonS3',
+        AccessKeyId     => '...',
+        SecretAccessKey => '...',
+        Bucket          => '...',
+    );
+
+=head1 DESCRIPTION
+
+This storage option places attachments in the S3 cloud file storage
+service.  The files are de-duplicated when they are saved; as such, if
+the same file appears in multiple transactions, only one copy will be
+stored in S3.
+
+Files in S3 B<must not be modified or removed>; doing so may cause
+internal inconsistency.  It is also important to ensure that the S3
+account used maintains sufficient funds for your RT's B<storage and
+bandwidth> needs.
+
+=head1 SETUP
+
+In order to use this storage type, 
+
+=over
+
+=item 1.
+
+Log into Amazon S3, L<https://aws.amazon.com/s3/>, as the account you wish
+to store files under.
+
+=item 2.
+
+Navigate to "Security Credentials" under your account name in the menu bar.
+
+=item 3.
+
+Open the "Access Keys" pane.
+
+=item 4.
+
+Click "Create New Access Key".
+
+=item 5.
+
+Copy the provided values for Access Key ID and Secret Access Key into
+ your F<RT_SiteConfig.pm> file:
+
+    Set(%ExternalStorage,
+        Type            => 'AmazonS3',
+        AccessKeyId     => '...', # Put Access Key ID between quotes
+        SecretAccessKey => '...', # Put Secret Access Key between quotes
+        Bucket          => '...',
+    );
+
+=item 6.
+
+Set up a Bucket for RT to use. You can either create and configure it
+in the S3 web interface, or let RT create one itself. Either way, tell
+RT what bucket name to use in your F<RT_SiteConfig.pm> file:
+
+    Set(%ExternalStorage,
+        Type            => 'AmazonS3',
+        AccessKeyId     => '...',
+        SecretAccessKey => '...',
+        Bucket          => '...', # Put bucket name between quotes
+    );
+
+=back
+
+=cut
+
 RT::Base->_ImportOverlays();
 
 1;

commit 9288f351243293571f8b887bf462f18a3b46d24d
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Wed May 27 17:47:43 2015 +0000

    This should be bold not comment

diff --git a/lib/RT/ExternalStorage/Dropbox.pm b/lib/RT/ExternalStorage/Dropbox.pm
index 0aa6e52..44557c8 100644
--- a/lib/RT/ExternalStorage/Dropbox.pm
+++ b/lib/RT/ExternalStorage/Dropbox.pm
@@ -140,7 +140,7 @@ service.  The files are de-duplicated when they are saved; as such, if
 the same file appears in multiple transactions, only one copy will be
 stored in Dropbox.
 
-Files in Dropbox C<must not be modified or removed>; doing so may cause
+Files in Dropbox B<must not be modified or removed>; doing so may cause
 internal inconsistency.  It is also important to ensure that the Dropbox
 account used has sufficient space for the attachments, and to monitor
 its space usage.

commit 9f3ce5f2a332e84db86be1ac4583887fc015da61
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Wed May 27 17:47:57 2015 +0000

    Add --verbose to rt-externalize-attachments

diff --git a/sbin/rt-externalize-attachments.in b/sbin/rt-externalize-attachments.in
index 9ca5d97..c93ff25 100644
--- a/sbin/rt-externalize-attachments.in
+++ b/sbin/rt-externalize-attachments.in
@@ -66,6 +66,19 @@ BEGIN { # BEGIN RT CMD BOILERPLATE
 
 }
 
+# Read in the options
+my %opts;
+use Getopt::Long;
+GetOptions( \%opts,
+    "help|h", "verbose|v",
+);
+
+if ($opts{'help'}) {
+    require Pod::Usage;
+    print Pod::Usage::pod2usage(-verbose => 2);
+    exit;
+}
+
 use RT -init;
 
 # Ensure we only run one of these processes at once
@@ -152,6 +165,10 @@ for my $class (qw/RT::Attachments RT::ObjectCustomFieldValues/) {
             );
 
             # Attempt to write that out
+            if ($opts{verbose}) {
+                RT->Logger->info("Storing $class $id");
+            }
+
             my ($key, $msg) = Store( $content );
             unless ($key) {
                 RT->Logger->error("Failed to store $class $id: $msg");
@@ -173,6 +190,10 @@ for my $class (qw/RT::Attachments RT::ObjectCustomFieldValues/) {
                 RT->Logger->error("Failed to update ContentEncoding of $class $id: $msg");
                 exit 2;
             }
+
+            if ($opts{verbose}) {
+                RT->Logger->info("Stored $class $id as $key");
+            }
         }
         $RT::Handle->dbh->commit;
 
@@ -194,5 +215,36 @@ sub Store {
     return ($key);
 }
 
+=head1 NAME
+
+rt-externalize-attachments - Move attachments from database to external storage
+
+=head1 SYNOPSIS
+
+    rt-externalize-attachments [options]
+
+=head1 OPTIONS
+
+This tool supports a few options. Most are for debugging.
+
+=over 8
+
+=item -h
+
+=item --help
+
+Display this documentation
+
+=item -v
+
+=item --verbose
+
+Log a message (at level "info") for each attachment, both before its
+upload begins and after it completes.
+
+=back
+
+=cut
+
 # don't remove; for locking (see call to flock above)
 __DATA__

commit d5d2770c1d1f1786642ff61dd66f4d1c6886293e
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Wed May 27 18:06:17 2015 +0000

    Explain _why_ we're not uploading to external storage

diff --git a/lib/RT/Attachment.pm b/lib/RT/Attachment.pm
index c7995b7..8b5e2d6 100644
--- a/lib/RT/Attachment.pm
+++ b/lib/RT/Attachment.pm
@@ -1183,19 +1183,23 @@ sub ShouldStoreExternally {
     my $type = $self->ContentType;
     my $length = $self->ContentLength;
 
-    return 0 if $length == 0;
-
     if ($type =~ m{^multipart/}) {
-        return 0;
-    } elsif ($type =~ m{^(text|message)/}) {
-        # If textual, we only store externally if it's _large_ (> 10M)
+        return (0, "attachment is multipart");
+    }
+    elsif ($length == 0) {
+        return (0, "zero length");
+    }
+    elsif ($type =~ m{^(text|message)/}) {
+        # If textual, we only store externally if it's _large_
         return 1 if $length > RT->Config->Get('ExternalStorageCutoffSize');
-        return 0;
-    } elsif ($type =~ m{^image/}) {
+        return (0, "text length ($length) does not exceed ExternalStorageCutoffSize (" . RT->Config->Get('ExternalStorageCutoffSize') . ")");
+    }
+    elsif ($type =~ m{^image/}) {
         # Ditto images, which may be displayed inline
         return 1 if $length > RT->Config->Get('ExternalStorageCutoffSize');
-        return 0;
-    } else {
+        return (0, "image size ($length) does not exceed ExternalStorageCutoffSize (" . RT->Config->Get('ExternalStorageCutoffSize') . ")");
+    }
+    else {
         return 1;
     }
 }
diff --git a/lib/RT/ObjectCustomFieldValue.pm b/lib/RT/ObjectCustomFieldValue.pm
index b9e1430..92a0e02 100644
--- a/lib/RT/ObjectCustomFieldValue.pm
+++ b/lib/RT/ObjectCustomFieldValue.pm
@@ -738,13 +738,17 @@ sub ShouldStoreExternally {
     my $type = $self->CustomFieldObj->Type;
     my $length = length($self->LargeContent || '');
 
-    return 0 if $length == 0;
+    return (0, "zero length") if $length == 0;
 
     return 1 if $type eq "Binary";
 
-    return 1 if $type eq "Image" and $length > RT->Config->Get('ExternalStorageCutoffSize');
+    if ($type eq "Image") {
+        # We only store externally if it's _large_
+        return 1 if $length > RT->Config->Get('ExternalStorageCutoffSize');
+        return (0, "image size ($length) does not exceed ExternalStorageCutoffSize (" . RT->Config->Get('ExternalStorageCutoffSize') . ")");
+    }
 
-    return 0;
+    return (0, "Only custom fields of type Binary or Image go into external storage (not $type)");
 }
 
 sub ExternalStoreDigest {
diff --git a/sbin/rt-externalize-attachments.in b/sbin/rt-externalize-attachments.in
index c93ff25..36dd3e9 100644
--- a/sbin/rt-externalize-attachments.in
+++ b/sbin/rt-externalize-attachments.in
@@ -155,7 +155,12 @@ for my $class (qw/RT::Attachments RT::ObjectCustomFieldValues/) {
         $RT::Handle->dbh->begin_work;
         while ( my $a = $attach->Next ) {
             $id = $a->id;
-            next unless $a->ShouldStoreExternally;
+
+            my ($ok, $why) = $a->ShouldStoreExternally;
+            if (!$ok) {
+                RT->Logger->info("Skipping $class $id because: $why");
+                next;
+            }
 
             # Explicitly get bytes (not characters, which ->$column would do)
             my $content = $a->_DecodeLOB(
diff --git a/t/externalstorage/basic.t b/t/externalstorage/basic.t
index c71935d..2fc5528 100644
--- a/t/externalstorage/basic.t
+++ b/t/externalstorage/basic.t
@@ -34,22 +34,29 @@ my @attachs = @{ $ticket->Transactions->First->Attachments->ItemsArrayRef };
 is scalar @attachs, 4, "Contains a multipart and two sub-parts";
 
 is $attachs[0]->ContentType, "multipart/mixed", "Found the top multipart";
-ok !$attachs[0]->ShouldStoreExternally, "Shouldn't store multipart part on disk";
+my ($ok, $msg) = $attachs[0]->ShouldStoreExternally;
+ok !$ok, "Shouldn't store multipart part on disk";
+like $msg, qr/attachment is multipart/, "Shouldn't store multipart part on disk";
 
 is $attachs[1]->ContentType, "text/plain", "Found the text part";
 is $attachs[1]->Content, 'test', "Can get the text part content";
 is $attachs[1]->ContentEncoding, "none", "Content is not encoded";
-ok !$attachs[1]->ShouldStoreExternally, "Won't store text part on disk";
+($ok, $msg) = $attachs[1]->ShouldStoreExternally;
+ok !$ok, "Won't store text part on disk";
+like $msg, qr/text length.*does not exceed/, "Won't store text part on disk";
 
 is $attachs[2]->ContentType, "image/special", "Found the image part";
 is $attachs[2]->Content, 'boo',  "Can get the image content";
 is $attachs[2]->ContentEncoding, "none", "Content is not encoded";
-ok !$attachs[2]->ShouldStoreExternally, "Won't store images on disk";
+($ok, $msg) = $attachs[2]->ShouldStoreExternally;
+ok !$ok, "Won't store images on disk";
+like $msg, qr/image size.*does not exceed/, "Won't store images on disk";
 
 is $attachs[3]->ContentType, "application/octet-stream", "Found the binary part";
 is $attachs[3]->Content, 'thing',  "Can get the binary content";
 is $attachs[3]->ContentEncoding, "none", "Content is not encoded";
-ok $attachs[3]->ShouldStoreExternally, "Will store binary data on disk";
+($ok, $msg) = $attachs[3]->ShouldStoreExternally;
+ok $ok, "Will store binary data on disk";
 
 my $dir = RT::Test::ExternalStorage->attachments_dir;
 ok !<$dir/*>, "Attachments directory is empty";

commit ef18fcbba21d075f9f3c45074bf02007c2e4e6ad
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Wed May 27 18:06:52 2015 +0000

    Filter should store externally in Perl, not SQL
    
        This reduces performance, but it improves visibility and flexibility.
        The code for answering "should store externally?" is no longer
        duplicated across Perl and SQL; this means overlays and extensions can
        adjust how to answer the question. Finally the improved visibility
        helps sysadmins because the skip reason can be logged (as opposed
        to skipping in the database).
    
        Performance should be fine because it will largely be dominated by
        uploading, and the high-water marks mean we won't re-examine
        attachments.

diff --git a/sbin/rt-externalize-attachments.in b/sbin/rt-externalize-attachments.in
index 36dd3e9..3e0a068 100644
--- a/sbin/rt-externalize-attachments.in
+++ b/sbin/rt-externalize-attachments.in
@@ -118,36 +118,8 @@ for my $class (qw/RT::Attachments RT::ObjectCustomFieldValues/) {
             VALUE           => 'external',
             ENTRYAGGREGATOR => 'AND',
         );
-        if ($class eq "RT::Attachments") {
-            $attach->_OpenParen('applies');
-            $attach->Limit(
-                FIELD     => 'ContentType',
-                OPERATOR  => 'NOT STARTSWITH',
-                VALUE     => $_,
-                SUBCLAUSE => 'applies',
-                ENTRYAGGREGATOR => "AND",
-            ) for "text/", "message/", "image/", "multipart/";
-            $attach->_CloseParen('applies');
-            $attach->Limit(
-                FUNCTION  => 'LENGTH(main.Content)',
-                OPERATOR  => '>',
-                VALUE     => RT->Config->Get('ExternalStorageCutoffSize'),
-                SUBCLAUSE => 'applies',
-                ENTRYAGGREGATOR => 'OR',
-            );
-        } else {
-            my $cfs = $attach->Join(
-                ALIAS1 => 'main',
-                FIELD1 => 'CustomField',
-                TABLE2 => 'CustomFields',
-                FIELD2 => 'id',
-            );
-            $attach->Limit(
-                ALIAS    => $cfs,
-                FIELD    => 'Type',
-                OPERATOR => 'IN',
-                VALUE    => [ qw(Binary Image) ],
-            );
+
+        if ($class eq "RT::ObjectCustomFieldValues") {
             $attach->{'find_expired_rows'} = 1;
         }
 

commit fccaa3f3b775779c9864535b664ff86bd49f0b16
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Wed May 27 18:11:18 2015 +0000

    Limit the transaction to each upload
    
        This way if upload #99 fails, we won't bother trying to re-upload
        1-98 since they are marked as external.

diff --git a/sbin/rt-externalize-attachments.in b/sbin/rt-externalize-attachments.in
index 3e0a068..8aefb14 100644
--- a/sbin/rt-externalize-attachments.in
+++ b/sbin/rt-externalize-attachments.in
@@ -124,7 +124,6 @@ for my $class (qw/RT::Attachments RT::ObjectCustomFieldValues/) {
         }
 
         $attach->RowsPerPage(100);
-        $RT::Handle->dbh->begin_work;
         while ( my $a = $attach->Next ) {
             $id = $a->id;
 
@@ -152,6 +151,8 @@ for my $class (qw/RT::Attachments RT::ObjectCustomFieldValues/) {
                 exit 1;
             }
 
+            $RT::Handle->dbh->begin_work;
+
             (my $status, $msg ) = $a->__Set(
                 Field => $column, Value => $key
             );
@@ -167,12 +168,12 @@ for my $class (qw/RT::Attachments RT::ObjectCustomFieldValues/) {
                 RT->Logger->error("Failed to update ContentEncoding of $class $id: $msg");
                 exit 2;
             }
+            $RT::Handle->dbh->commit;
 
             if ($opts{verbose}) {
                 RT->Logger->info("Stored $class $id as $key");
             }
         }
-        $RT::Handle->dbh->commit;
 
         last unless $attach->Count;
     }

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


More information about the rt-commit mailing list