New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Rackspace Cloud Files TempURL support #72

Closed
wants to merge 25 commits into
base: trunk
from

Conversation

Projects
None yet
2 participants
@shawnps

shawnps commented Aug 7, 2012

No description provided.

@Kami

This comment has been minimized.

Show comment
Hide comment
@Kami

Kami Aug 7, 2012

We should probably set default value for timeout (e.g. 60 seconds).

We should probably set default value for timeout (e.g. 60 seconds).

This comment has been minimized.

Show comment
Hide comment
@shawnps
Owner

shawnps replied Aug 7, 2012

@Kami

This comment has been minimized.

Show comment
Hide comment
@Kami

Kami Aug 7, 2012

Should wrap it in try / except and throw a more user-friendly exception if temp_url_key is not available (KeyError`). For example "You need to set account meta data key using ex_set_metadata, before you can use this method.

Should wrap it in try / except and throw a more user-friendly exception if temp_url_key is not available (KeyError`). For example "You need to set account meta data key using ex_set_metadata, before you can use this method.

This comment has been minimized.

Show comment
Hide comment
@shawnps
Owner

shawnps replied Aug 7, 2012

@Kami

This comment has been minimized.

Show comment
Hide comment
@Kami

Kami Aug 7, 2012

Don't do that, use urllib.urlencode. It also seems like you are missing ? after /.

Don't do that, use urllib.urlencode. It also seems like you are missing ? after /.

This comment has been minimized.

Show comment
Hide comment
@shawnps
Owner

shawnps replied Aug 7, 2012

Show outdated Hide outdated libcloud/storage/drivers/cloudfiles.py
return response.status in [httplib.OK, httplib.NO_CONTENT,
httplib.CREATED, httplib.ACCEPTED]
def ex_get_container_temp_url(self, container, method, timeout=60):

This comment has been minimized.

@Kami

Kami Aug 7, 2012

Member

Look at the docs (http://docs.rackspace.com/files/api/v1/cf-devguide/content/Create_TempURL-d1a444.html), you also need to specify an object prefix when creating a temp URL for a container.

@Kami

Kami Aug 7, 2012

Member

Look at the docs (http://docs.rackspace.com/files/api/v1/cf-devguide/content/Create_TempURL-d1a444.html), you also need to specify an object prefix when creating a temp URL for a container.

This comment has been minimized.

@shawnps

shawnps Aug 7, 2012

I saw that, yeah. That's only for PUTs though, right? And I don't see where the prefix goes, is it a query param?

@shawnps

shawnps Aug 7, 2012

I saw that, yeah. That's only for PUTs though, right? And I don't see where the prefix goes, is it a query param?

This comment has been minimized.

@Kami

Kami Aug 7, 2012

Member

Not sure, documentation isn't totally cleared - would need to check the code.

@Kami

Kami Aug 7, 2012

Member

Not sure, documentation isn't totally cleared - would need to check the code.

Show outdated Hide outdated libcloud/storage/drivers/cloudfiles.py
import os
from time import time
from urllib import urlencode

This comment has been minimized.

@Kami

Kami Aug 7, 2012

Member

Need to use one from libcloud.utils.py3.

@Kami

Kami Aug 7, 2012

Member

Need to use one from libcloud.utils.py3.

This comment has been minimized.

@@ -674,6 +679,9 @@ def _v1_MossoCloudFS(self, method, url, body, headers):
'x-account-object-count': 400,
'x-account-bytes-used': 1234567
})
elif method == 'POST':

This comment has been minimized.

@Kami

Kami Aug 7, 2012

Member

Also assert that the correct temp_url_sig and temp_url_expires values are in the request headers.

@Kami

Kami Aug 7, 2012

Member

Also assert that the correct temp_url_sig and temp_url_expires values are in the request headers.

This comment has been minimized.

@shawnps

shawnps Aug 7, 2012

Hmm, for POSTing to set the account metadata temp URL key? temp_url_sig and temp_url_expires are query params in the temporary URL that you give to the user.

@shawnps

shawnps Aug 7, 2012

Hmm, for POSTing to set the account metadata temp URL key? temp_url_sig and temp_url_expires are query params in the temporary URL that you give to the user.

This comment has been minimized.

@Kami

Kami Aug 7, 2012

Member

Ah, sorry, thought that this was for get_temp_url stuff.

@Kami

Kami Aug 7, 2012

Member

Ah, sorry, thought that this was for get_temp_url stuff.

This comment has been minimized.

@shawnps

shawnps Aug 7, 2012

No problem. I added other tests for that stuff here

@shawnps

shawnps Aug 7, 2012

No problem. I added other tests for that stuff here

Show outdated Hide outdated libcloud/storage/drivers/cloudfiles.py
Cloud Files account using \
ex_set_account_metadata_temp_url_key before \
you can use this method.")
hmac_body = "%s\n%s\n%s" % (method, expires, path)

This comment has been minimized.

@Kami

Kami Aug 7, 2012

Member

Style bike shed: use single quotes

@Kami

Kami Aug 7, 2012

Member

Style bike shed: use single quotes

This comment has been minimized.

Show outdated Hide outdated libcloud/storage/drivers/cloudfiles.py
path = self.connection.request_path + '/' + container.name
try:
key = self.ex_get_meta_data()['temp_url_key']
key != None

This comment has been minimized.

@Kami

Kami Aug 7, 2012

Member

Did you want to assert or something here? Just doesn't that won't cause an exception to be thrown. You can do like assert key is not None

@Kami

Kami Aug 7, 2012

Member

Did you want to assert or something here? Just doesn't that won't cause an exception to be thrown. You can do like assert key is not None

This comment has been minimized.

Show outdated Hide outdated libcloud/storage/drivers/cloudfiles.py
try:
key = self.ex_get_meta_data()['temp_url_key']
assert key is not None
except:

This comment has been minimized.

@Kami

Kami Aug 8, 2012

Member

IRC, the correct Pythonic way is doing except Exception:

@Kami

Kami Aug 8, 2012

Member

IRC, the correct Pythonic way is doing except Exception:

This comment has been minimized.

Show outdated Hide outdated libcloud/test/storage/test_cloudfiles.py
self.driver.ex_get_meta_data.return_value = {'container_count': 1,
'object_count': 1,
'bytes_used': 1,
'temp_url_key': "foo"}

This comment has been minimized.

@Kami

Kami Aug 8, 2012

Member

missed the quote :P

@Kami

Kami Aug 8, 2012

Member

missed the quote :P

This comment has been minimized.

except Exception, change all double quotes to single quotes, add test…
…_ex_get_object_temp_url_no_key_raises_key_error
Show outdated Hide outdated libcloud/storage/drivers/cloudfiles.py
from libcloud.utils.py3 import httplib
from libcloud.utils.py3 import urllib

This comment has been minimized.

@Kami

Kami Aug 8, 2012

Member

You should use libcloud.utils.py3.urlencode.

@Kami

Kami Aug 8, 2012

Member

You should use libcloud.utils.py3.urlencode.

This comment has been minimized.

Show outdated Hide outdated libcloud/test/storage/test_cloudfiles.py
container=container, meta_data=None,
driver=self)
exception_string = 'You must first set the X-Account-Meta-Temp-URL-Key header on your Cloud Files account using ex_set_account_metadata_temp_url_key before you can use this method.'
with self.assertRaisesRegexp(KeyError, exception_string):

This comment has been minimized.

@Kami

Kami Aug 8, 2012

Member

That's not available in version < 2.7 so you can't do that.

@Kami

Kami Aug 8, 2012

Member

That's not available in version < 2.7 so you can't do that.

This comment has been minimized.

@Kami

This comment has been minimized.

Show comment
Hide comment
@Kami

Kami Aug 8, 2012

Member

Thanks. Merged into trunk (http://svn.apache.org/viewvc?view=revision&revision=1370664) with some changes:

  • Default method to GET
  • Fix the HMAC calculation so it works with Python 3.x
Member

Kami commented Aug 8, 2012

Thanks. Merged into trunk (http://svn.apache.org/viewvc?view=revision&revision=1370664) with some changes:

  • Default method to GET
  • Fix the HMAC calculation so it works with Python 3.x
@Kami

This comment has been minimized.

Show comment
Hide comment
@Kami

Kami Sep 5, 2012

Member

This has been merged, can you please close this PR?

Member

Kami commented Sep 5, 2012

This has been merged, can you please close this PR?

@shawnps shawnps closed this Sep 5, 2012

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