Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Add Amazon S3 Mover #28

Merged
merged 13 commits into from Aug 6, 2011

Conversation

Projects
None yet
3 participants
Contributor

dse commented Jul 13, 2011

Greetings!

John Durkin john@j2d3.com has developed an Amazon S3 mover and told me that it works for him. Because he's too pressed for time himself, I'm stepping in to submit the code for inclusion in Bricolage. You'll find the changes (four modified files and one new one) in the "dev_amazon_s3" branch in my fork.

If you have any further questions please talk to both him and me.

Thanks!

@dse dse Add Amazon S3 mover.
Implemented as a new move method.

Code by John Durkin <john@j2d3.com>.

Merge into bricolage github by Darren Embry <darren@rfxtechnologies.com>.
ea10d15

@theory theory commented on the diff Jul 13, 2011

lib/Bric/Util/Trans/S3.pm
+=head1 Synopsis
+
+ use Bric::Util::Trans::S3
+
+=head1 Description
+
+The distribution API uses this class to distribute resources to AWS S3 buckets.
+
+=cut
+
+################################################################################
+# Dependencies
+################################################################################
+# Standard Dependencies
+use strict;
+use Net::Amazon::S3;
@theory

theory Jul 13, 2011

Owner

These need to be added to the list of optionally modules in Bric::Config.

@theory

theory Jul 15, 2011

Owner

Also in Bric::Admin:

=item Net::Amazone::S3 (optional)

Put it below this line:

=item Net::SSH2 0.28 (optional)

@theory theory and 1 other commented on an outdated diff Jul 13, 2011

lib/Bric/Dist/ServerType.pm
my $and_dav = ENABLE_WEBDAV_MOVER ? '' : q{AND key_name <> 'webdav'};
my $sel = prepare_ca(qq{
SELECT disp_name
FROM class
- WHERE distributor = '1' $and_sftp $and_dav
@theory

theory Jul 13, 2011

Owner

Whats this for?

@dse

dse Jul 14, 2011

Contributor

On Wed, Jul 13, 2011 at 19:25, theory
reply@reply.github.com
wrote:

     my $and_dav = ENABLE_WEBDAV_MOVER ? '' : q{AND key_name <> 'webdav'};
     my $sel = prepare_ca(qq{
         SELECT disp_name
         FROM   class

  •        WHERE  distributor = '1' $and_sftp $and_dav

Whats this for?

That change added a space at the end of a line in the file.
It must have been inadvertent on John's part.
Forgot to take it out, sorry.

@theory

theory Jul 14, 2011

Owner

No no, I mean the my $and_s3 line that's commented it out. Is it not needed?

@dse

dse Jul 14, 2011

Contributor

On Thu, Jul 14, 2011 at 12:06, theory
reply@reply.github.com
wrote:

     my $and_dav = ENABLE_WEBDAV_MOVER ? '' : q{AND key_name <> 'webdav'};
     my $sel = prepare_ca(qq{
         SELECT disp_name
         FROM   class

  •        WHERE  distributor = '1' $and_sftp $and_dav

No no, I mean the my $and_s3 line that's commented it out. Is it not needed?

Ah, that... I don't think so. I'm also sending this response to John
Durkin to see if he has any further thoughts...

@dse

dse Jul 15, 2011

Contributor

On Thu, Jul 14, 2011 at 12:06, theory wrote:

No no, I mean the my $and_s3 line that's commented it out. Is it not needed?

On Thu, Jul 14, 2011 at 12:17, Darren Embry dse@webonastick.com wrote:

Ah, that... I don't think so.  I'm also sending this response to John
Durkin to see if he has any further thoughts...

Well since I decided to add ENABLE_S3_MOVER I've realized that line
needs to be added now. I'll let you know when my current round of
changes is in my branch on github.

@theory theory commented on an outdated diff Jul 14, 2011

lib/Bric/Util/Trans/S3.pm
+=head2 Private Functions
+
+NONE.
+
+=cut
+
+1;
+__END__
+
+=head1 Notes
+
+NONE.
+
+=head1 Author
+
+David Wheeler E<lt>david@justatheory.comE<gt>
@theory

theory Jul 14, 2011

Owner

Someone else needs to be credited here. :-)

Owner

theory commented Jul 14, 2011

I added a few notes, as you can see, but I'd also like to see some tests. See t/Bric/Util/Trans/Mail/Test.pm and t/Bric/Util/Trans/Mail/DevTest.pm for how the Mail mover is tested depending on what's installed and such.

Thanks!

Contributor

dse commented Jul 15, 2011

Okay, this should be good. The test created in 2af2a5d is not really a stub (I misspoke in the commit msg) but it only checks for the prerequisite module, similar to the one for the SFTP mover.

@theory theory commented on an outdated diff Jul 15, 2011

README.Debian
@@ -51,6 +51,15 @@ There was no Net::SFTP installer from Debian, so proceed with
$ sudo cpan install Net::SFTP
+Amazon S3 Mover
+---------------
+
+If you use the optional SFTP mover, you have to install Net::SFTP by

@theory theory commented on an outdated diff Jul 15, 2011

lib/Bric/Dist/Action/Mover.pm
@@ -35,11 +35,16 @@ use Bric::App::Event qw(log_event);
use Bric::Util::Fault qw(throw_ap);
use Bric::Util::Trans::FS;
use Bric::Util::Trans::FTP;
+use Bric::Util::Trans::S3;
@theory

theory Jul 15, 2011

Owner

You don't need this line with the if (ENABLE_S3_MOVER) block below.

@theory theory commented on an outdated diff Jul 15, 2011

lib/Bric/Util/Trans/S3.pm
+ next unless $s->is_active;
+ my $s3 = Net::Amazon::S3->new({
+ aws_access_key_id => $s->get_login,
+ aws_secret_access_key => $s->get_password,
+ retry => 1,
+ }) or
+ throw_gen
+ error => 'fail',
+ payload => 'login: ' . $s->get_login . "\n" . 'password: ' . $s->get_password;
+ # Instantiate the S3 bucket
+ my $bucket = $s3->bucket($s->get_host_name);
+ foreach my $res (@$resources) {
+ # Get the source and destination paths for the resource.
+ my $src = $res->get_tmp_path || $res->get_path;
+ my $dest = $s->get_doc_root . substr $res->get_uri, 1;
+ my $mime = mimetype($src);
@theory

theory Jul 15, 2011

Owner

No need to use File::MimeInfo here; Just use

$res->get_media_type;

@theory theory commented on the diff Jul 15, 2011

t/Bric/Util/Trans/S3/Test.pm
@@ -0,0 +1,23 @@
+package Bric::Util::Trans::S3::Test;
@theory

theory Jul 15, 2011

Owner

It's a start, though I'd like to see more comprehensive testing soon.

Owner

theory commented Jul 15, 2011

Loving the ability to comment on stuff here. Anyway, if you make the changes suggested in my comments, I'll pull. :-)

Owner

phillipadsmith commented Jul 26, 2011

Hey there dse & theory: What's the status of this pull request?

Owner

theory commented Jul 26, 2011

Merge it!

@theory theory added a commit that referenced this pull request Aug 6, 2011

@theory theory Merge pull request #28 from dse/dev_amazon_s3
Add Amazon S3 Mover. Thanks to John Durkin and Darren Embry.
631e289

@theory theory merged commit 631e289 into bricoleurs:master Aug 6, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment