On Tue, 2011-01-25 at 22:04 +0200, [email protected] wrote:
> From: marios <[email protected]>
ACK, with a few nits.
> diff --git a/server/lib/deltacloud/drivers/azure/azure_driver.rb
> b/server/lib/deltacloud/drivers/azure/azure_driver.rb
> index abbd353..8d7b1e6 100644
> --- a/server/lib/deltacloud/drivers/azure/azure_driver.rb
> +++ b/server/lib/deltacloud/drivers/azure/azure_driver.rb
> @@ -94,23 +94,28 @@ class AzureDriver < Deltacloud::BaseDriver
> #--
> # Create Blob
> #--
> - def create_blob(credentials, bucket_id, blob_id, blob_data, opts=nil)
> + def create_blob(credentials, bucket_id, blob_id, blob_data, opts={})
> azure_connect(credentials)
> - #get a handle to the bucket in order to put there
> - the_bucket = WAZ::Blobs::Container.find(bucket_id)
> - the_bucket.store(blob_id, blob_data[:tempfile], blob_data[:type])
> + #insert azure-specific header for user metadata ... x-ms-meta-kEY = VALUE
> + opts.gsub_keys("HTTP_X_Deltacloud_Blobmeta_", "x-ms-meta-")
> + safely do
> + #get a handle to the bucket in order to put there
> + the_bucket = WAZ::Blobs::Container.find(bucket_id)
> + the_bucket.store(blob_id, blob_data[:tempfile], blob_data[:type], opts)
> + end
> Blob.new( { :id => blob_id,
> :bucket => bucket_id,
> :content_lengh => blob_data[:tempfile].length,
> :content_type => blob_data[:type],
> - :last_modified => ''
> + :last_modified => '',
> + :user_metadata => opts.select{|k,v| k.match(/^x-ms-meta-/)}
This sets user_metadata to an array of pairs, i.e. sth like [[:a, 1],
[:b, 2]] instead of a hash. Also, the key names will still have the
x-ms-meta prefix - shouldn't that be stripped ?
> diff --git a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> index 7a4b394..c9e5319 100644
> --- a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> +++ b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> @@ -354,25 +354,29 @@ module Deltacloud
> #--
> # Create Blob
> #--
> - def create_blob(credentials, bucket_id, blob_id, data = nil, opts =
> nil)
> + def create_blob(credentials, bucket_id, blob_id, data = nil, opts =
> {})
> s3_client = new_client(credentials, :s3)
> #data is a construct with the temporary file created by server
> @.tempfile
> #also file[:type] will give us the content-type
> res = nil
> # File stream needs to be reopened in binary mode for whatever
> reason
> file = File::open(data[:tempfile].path, 'rb')
> + #insert ec2-specific header for user metadata ... x-amz-meta-KEY =
> VALUE
> + opts.gsub_keys('HTTP_X_Deltacloud_Blobmeta_', 'x-amz-meta-')
> + opts["Content-Type"] = data[:type]
> safely do
> res = s3_client.interface.put(bucket_id,
> blob_id,
> file,
> - {"Content-Type" => data[:type]})
> + opts)
> end
> #create a new Blob object and return that
> Blob.new( { :id => blob_id,
> :bucket => bucket_id,
> :content_length => data[:tempfile].length,
> :content_type => data[:type],
> - :last_modified => ''
> + :last_modified => '',
> + :user_metadata => opts.select{|k,v|
> k.match(/^x-amz-meta-/)}
Same comment as for Azure.
> diff --git a/server/views/blobs/new.html.haml
> b/server/views/blobs/new.html.haml
> index feb94e5..a9817d5 100644
> --- a/server/views/blobs/new.html.haml
> +++ b/server/views/blobs/new.html.haml
> @@ -4,7 +4,23 @@
> %label
> Blob Name:
> %input{ :name => 'blob_id', :size => 512}/
> + %label
> Blob Data:
> - %input{ :type => "file", :name => 'blob_data', :size => 50}/
> %br
> + %input{ :type => "file", :name => 'blob_data', :size => 50}/
> + %br
> + %br
> + %input{ :type => "hidden", :name => "meta_params", :value => "0"}
> + %a{ :href => "javascript:;", :onclick => "more_fields();"} Add Metadata
> + %div{ :id => "metadata_holder", :style => "display: none;"}
> + %label
> + Metadata Name:
Minor nit: shouldn't we call that a metadata key ? Seems that's the
general nomenclature.
> diff --git a/server/views/blobs/show.html.haml
> b/server/views/blobs/show.html.haml
> index e5c2cee..29025c0 100644
> --- a/server/views/blobs/show.html.haml
> +++ b/server/views/blobs/show.html.haml
> @@ -19,6 +19,9 @@
> %dt Content
> %dd
> =link_to 'Blob content', bucket_url(@blob.bucket) + '/' + @blob.id +
> '/content'
> + %dt User_Metadata
> + %dd
> + = @blob.user_metadata.inspect
Another nit: maybe we should format this more friendly than with
inspect ;)
> diff --git a/server/views/blobs/show.xml.haml
> b/server/views/blobs/show.xml.haml
> index bc499ca..1b9ea55 100644
> --- a/server/views/blobs/show.xml.haml
> +++ b/server/views/blobs/show.xml.haml
> @@ -1,7 +1,9 @@
> !!! XML
> %blob{:href => bucket_url(@blob.bucket) + '/' + @blob.id, :id => @blob.id}
> - - @blob.attributes.select{ |attr| attr!=:id}.each do |attribute|
> + - @blob.attributes.select{ |attr| (attr!=:id && attr!=:user_metadata)
> }.each do |attribute|
> - unless attribute == :content
> - haml_tag(attribute, :<) do
> - haml_concat @blob.send(attribute)
> + - @blob.user_metadata.each do |k, v|
> + %user_metadata{:name => k, :value => v}
Do we need to worry about the size of v and possibly that it might
contain illegal characters here ? Might be safer to make the value the
content of the tag, possibly in a CDATA section.
Also, I'd prefer if the metadata is inside a container tag, like
<user_metadata>
<entry key="artist">John Lennon</entry>
...
<user_metadata>
David
