Skip to content

Instantly share code, notes, and snippets.

@myronmarston
Forked from waltjones/aws_s3.rb
Created April 12, 2011 21:39
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save myronmarston/916480 to your computer and use it in GitHub Desktop.
Save myronmarston/916480 to your computer and use it in GitHub Desktop.
module Starfish
module AwsS3
# S3Archive initializes and provides archival-oriented access to an S3 bucket.
class S3Archive
def initialize
config = YAML.load_file("#{File.expand_path('~')}/.ec2/aws-secret.yml")
AWS::S3::Base.establish_connection!(
:access_key_id => config['aws']['access_key'],
:secret_access_key => config['aws']['secret_access_key']
)
end
def bucket(name, create=false)
AWS::S3::Bucket.find(name) || AWS::S3::Bucket.create(name)
end
# Accepts a string or IO object. Compresses string data with zlib deflate.
# Zlib::GzipReader generates an IO object compatible with this method.
def put(bucket_name, key, data, opts={})
options = {:content_type => 'binary/octet-stream'}.merge(opts)
data = StringIO.new(Zlib::Deflate::deflate(data)) if data.is_a?(String)
AWS::S3::S3Object.store(key, data, bucket_name, options)
end
# Accepts a string or IO object. Expands string data with zlib inflate.
# Zlib::GzipWriter generates an IO object compatible with this method.
def get(bucket_name, key, io=nil, &block)
if io.respond_to?(:write)
AWS::S3::S3Object.stream(key, bucket_name) do |chunk|
io.write chunk
end
elsif block
AWS::S3::S3Object.stream(key, bucket_name, {}, &block)
else
Zlib::Inflate::inflate(AWS::S3::S3Object.value(key, bucket_name))
end
end
end
end
end
# Sign up for AWS and get your keys: http://aws.amazon.com/
# Keep your secrets out of the project repo. :)
aws:
access_key: XXXXXXXXXXXXXXX
secret_access_key: XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
account: XXXXXXXXXXXX
module Starfish
module Pipeline
# SqsQueue initializes and provides basic access to an AWS SQS queue.
class SqsQueue
attr_reader :queue, :name, :sqs
def initialize(name)
config = YAML.load_file("#{File.expand_path('~')}/.ec2/aws-secret.yml")
@sqs = RightAws::SqsGen2.new(
config['aws']['access_key'],
config['aws']['secret_access_key'],
{:logger => Rails.logger})
@name = name
@queue = @sqs.queue(@name, true, "1")
end
# This method enqueues a message, assumed to be a Ruby hash object,
# which is serialized to JSON.
def send(message)
@queue.send_message(JSON[message])
end
# This method both receives and deletes the message, assumed to be a JSON
# object, and converts to a Ruby hash object.
def pop
if message = @queue.pop
JSON[message.to_s]
end
end
# This method peeks at a message and triggers the visibility timeout,
# but doesn't remove/delete the message.
def receive
if message = @queue.receive
JSON[message.to_s]
end
end
end
end
end
@myronmarston
Copy link
Author

Changes I've made:

  • aws_s3.rb:8: you were using string interpolation and concatenation. It's pretty unidiomatic ruby to mix-and-match these like this. Generally you should use one or the other but not both (I chose interpolation here).
  • aws_s3.rb:11: You had an extra bracket here.

@myronmarston
Copy link
Author

aws_s3.rb:16: or is kinda a confusing ruby keyword. Because of precedence rules, it has some gotchas and is really meant to be used for control flow. || is generally a better, more clear operator to use.

@myronmarston
Copy link
Author

Is S3Archive#bucket necessary to be included in this example code? I don't see anything that uses this method so it might be getter to trim it.

@myronmarston
Copy link
Author

aws_s3.rb:8: I just noticed that the ||= operator here is a bit confusing, because it's the initalizemethod, and @config will always be nil when it is called. So I changed it to = since the || is never used.

@myronmarston
Copy link
Author

aws_s3.rb:23: I changed data.class == String to data.is_a?(String) as I would consider that to be the more idiomatic way.

@myronmarston
Copy link
Author

I think it's neat that S3Archive#get has multiple ways of using it (i.e. passing an io object, passing a block, or just letting it return the raw data), but I think it adds confusion to the method and gets in the way of the example a bit. Even though that may be useful in your code base, the goal with examples here should be to pare them down to the smallest examples that will make sense to conference attendees.

@myronmarston
Copy link
Author

In S3Archive#initialize, there's no need to make the config variable and instance variable as you are not referencing it outside of that method. You might as well just make it a local variable (which I've already done).

@myronmarston
Copy link
Author

  • In SqsQueue#initialize, you mixed interpolation & concatenation again. I fixed it.
  • Again, config can just be a local variable. It doesn't need to be an instance variable (which saves memory--the garbage collector can reclaim it after initialize runs).
  • Is it really necessary to expose queue, name and sqs to consumers of SqsQueue? I thought the point was that SqsQueue wraps SQS. I'd prefer to not expose those so that it forces all consumers to use the interface you've provided.
  • I'd be careful naming your method send since that's a method defined on all ruby objects with the specific meaning of calling the named method. You've overridden it here to mean something else, which could be a source of bugs later on. Maybe enqueue would be a better name?

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