Rackspace storage metadata

[pastorius]

I needed support for object level metadata on Rackspace Cloud Storage. I assume you're open to this based on AWS::File#metadata?

I added some tests as well, but only for what I changed. Let me know if there's anything else I can do to get this patch in. Shindo was definitely new to me. Not sure if I'm using it correctly.

[geemus]

@pastorius - seems good I think. Shindo is new to everybody (I made the mistake of writing a test framework, oops).

@bradgignac @brianhartsock - thoughts?

[brianhartsock]

Using this means you couldn't add metadata by doing file.metadata['key'] = 'value'. Also, you are exposing users to the X-Object-Meta- portion of the meta-data which kind of stinks. It would be nice to abstract that complexity away entirely since it is definitely vendor specific.

Would probably be better if metadata was stored as an instance variable and loaded from attributes upon first access.

[pastorius]

Totally agree. I was just trying not to take too much liberty in diverging from the AWS implementation.

[brianhartsock]

@pastorius thanks a bunch for your work! I have a few comments. Let me know if you have any questions on them. Happy to help.

[brianhartsock]

@geemus I checked out the AWS version of meta-data and this pull request is pretty comparable. I have some recommended improvements but I'm not for sure it is a good idea to diverge from the AWS implementation too much. Let me know your thoughts.

[pastorius]

@geemus @brianhartsock I've made a few notes. I guess now I'm just waiting to get confirmation that you're comfortable with diverging from the AWS implementation in this way.

[geemus]

@brianhartsock - thanks for being conscientious about diversion. I think we should do the sane/safe thing (and update AWS if need be). I think AWS was done as simplest that would work, so it may be due for review. We should do the right thing here and see about matching in AWS in a backwards compatible way (if possible). Once you guys settle on something perhaps open a ticket describing the changes that need to occur in AWS so somebody can find them and tackle it? Thanks!

[pastorius]

@geemus It seems like there are at least a couple cases here.

If I call File#metadata and the file already exists in storage, I think it should load the metadata. See the following:

f = directory.files.get(my_key)
f.metadata    #should be loaded if file was previously saved with metadata 

And if I call #metadata on a new file, as in below, it seems like I should just start with an empty hash like we've been discussing, and then saving should convert my metadata hash to the appropriate headers and include those in the request.

f = directory.files.new({
  :key    => 'resume.html',
  :body   => 'improvements'
})
f.metadata[:foo] = 'bar'
f.save

Do you agree? If so, is there some way to tell if the file already exists in storage from within the File class itself? Or do I call collection.head to check for existence?

[geemus]

@pastorius makes sense. I'd check to see if last-modified is defined probably. If it is you can pretty reasonably assume the file exists (as end users really shouldn't be setting this themselves). etag fits in that category of attributes as well. Make sense?

[pastorius]

@geemus Sure does. I'll let you know if I have any other questions. Thanks.

On Nov 15, 2012, at 8:37 AM, Wesley Beary notifications@github.com wrote:

@pastorius makes sense. I'd check to see if last-modified is defined probably. If it is you can pretty reasonably assume the file exists (as end users really shouldn't be setting this themselves). etag fits in that category of attributes as well. Make sense?

—
Reply to this email directly or view it on GitHub.

[pastorius]

@geemus

Updated to support metadata using the indexer syntax and encapsulated the X-Object-Meta- header prefix.

Not sure how smart the conversion between hash key and HTTP header needs to be. I'd prefer to use TSTTCPW. Right now it converts symbols or strings with underscores and/or dashes, and supports compound key names. That's way more than I need though.

Let me know what you think.

[brianhartsock]

LGTM

@geemus thoughts? Opened #1288 to discuss changes to the amazon metadata implementation.

[pastorius]

@brianhartsock Cool.

One thing that still smells to me is all the logic related to converting metadata to and from HTTP headers buried in Fog::Storage::Rackspace::File. It feels like a violation of the SRP, and if every attribute in File were to do that, that class would get really busy really quick. It was painful testing those methods -- I found myself wishing they could have been public on some class somewhere so that I could test them without all the model plumbing. I might suggest extracting all of metadata's attendant conversion methods to a separate class (or maybe a good candidate for a module?), but perhaps revisiting Fog::Storage::AWS::File will help shed some light on the best way to do that.

I'm open to any feedback, and happy to offer help and opinions where you all think it makes sense.

[geemus]

@pastorius - looks good. It does seem messy, but I'm reticent to try and figure out a good abstraction from a single example. Probably would be good to get the AWS side updated also and then figure out how to do that in a more common fashion (or at least that is my initial inclination). Hopefully that can prevent making mistakes based on simple oversight due to only having the single example. Anyway, I think this is good enough for our purposes currently.

@brianhartsock - feel free to merge it when you are ready.

[pastorius]

@geemus Yup, makes sense. Thanks!

[geemus]

Thanks!