Skip to content
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

Rackspace CDN methods #96

Closed
wants to merge 6 commits into from
Closed

Conversation

jcrugzz
Copy link
Contributor

@jcrugzz jcrugzz commented Mar 7, 2013

Thought we could continue the discussion from #87 here @indexzero. I got all the container methods working fine, just ran into a hiccup with the upload/download methods due to how I have to get the storageUrl. I'll look into how I can extract the buffering code from the base.Client.request method to be able to reuse that for the upload/download. @cronopio any ideas? I just wanted to get some eyes on the code in the mean time. Let me know what you guys think.

Tests/Docs will also be forthcoming.

@indexzero
Copy link
Member

@cronopio Thoughts on this?

@kenperkins
Copy link
Member

I'd like to review this before we look at accepting this. I've got a PR in progress that fundamentally changes the underlying rackspace auth to use v2 auth (openstack based) that may impact this.

@jcrugzz
Copy link
Contributor Author

jcrugzz commented May 9, 2013

@kenperkins Yea a lot probably has changed and time has not allowed me a chance to see how everything in the repo has shifted yet. Its kind of hacky because it was playing games with the serviceUrl and doing some manual auth in order to do that. Not ideal in any sense. Let me know when you get the chance to review it and we can work from there.

@kenperkins
Copy link
Member

Thinking out loud here: at the abstract level, all CDN containers must fundamentally be a non-CDN container as well, so should there be a more tight coupling here? Should the user even know there's a different API with the provider for the bucket? or should they just be extension methods on the normal storage provider?

@jcrugzz
Copy link
Contributor Author

jcrugzz commented May 10, 2013

Yes they are all technically storage containers. I guess it depends if just using a storageClient with a specified option or flag is better than having it namespaced in someway from a usability standpoint. @kenperkins I'd love to see the possible api's for it that are bouncing around in your head :)

@bstahlhood
Copy link

@kenperkins Hey Ken. Now that you have merged in the OpenStack conversion, do you plan on getting full CDN support integrated soon?

@kenperkins
Copy link
Member

I've been thinking a lot about this, and I'm not convinced having the CDN be a different provider is the right way to go. I'm thinking about extending the current storage provider to have the CDN extension methods directly off of the containers, as you can't have a cdn container without a proper storage container backing it.

Does that make sense? If so, yes, I'm looking at that here in the next week or 3.

@bstahlhood
Copy link

@kenperkins Yes! I always thought it was nutty to have to create a separate client just do to CDN calls. This makes a lot of sense and welcome the change. Thank you for such a quick response and thanks for the work.

@kenperkins
Copy link
Member

I'm trying to get my head around the options for presenting a unified model to a developer for a storage container. My goal is to not have the caller worry/deal with the fact that CDN is a different endpoint as far as rackspace is concerned.

The problem stems from the fact that a container may be optionally CDN enabled.

Take for example this scenario in fake synchronous javascript:

var containers = client.getContainers({ limit: 1000, offset: 'some-container-name' });

Given this list of containers, how do you know which of these are CDN enabled? As I see it, there are no less than 4 options:

  • Make an implicit follow on query for every container in the result set to the CDN endpoint to load the CDN properties. This could be a nightmare if you have 10k containers in your result set.
  • Make the follow on query a function of options passed to the getContainers call, this would still result in a potentially massive number of queries:
var containers = client.getContainers({
    limit: 1000,
    offset: 'some-container-name', 
    loadCDNAttributes: true });
  • Make loading CDN attributes an explicit call on a given container:
container.loadCDNMetadata();
  • Provide no model level reconciliation between a CDN container and a storage container. I'm not in favor of this approach as there must be a backing storage container for any CDN container, and you must have a storage container in hand to enable a CDN container.

I think we'll probably want to do explicit loading from an instance but it just feels .... undesirable. Thoughts?

@EdLeafe
Copy link

EdLeafe commented May 31, 2013

I'm not claiming that this is the best way for other SDKs, but pyrax loads the CDN information when it creates a container object. So given a container cont, the value of cont.cdn_enabled will be True or False, depending on whether it is CDN-enabled or not, and cont.cdn_uri will contain the URI for accessing the objects in the container via CDN; it will be None if the container is not CDN-enabled.

I did it this way because it felt the most natural way for a developer using the SDK to access the information.

@kenperkins
Copy link
Member

@EdLeafe So to be clear: you implicitly load CDN attributes when you create a container model.

@jcrugzz
Copy link
Contributor Author

jcrugzz commented May 31, 2013

@kenperkins yea that was one of the issues I came across (checkout the first example of #87 for reference). What I mention is, what if you just want to deal with JUST CDN containers or JUST storage containers? While we would be able to enable the first case with a flag like what is done in cloudfiles, there is no way to do the second without filtering the list based on which ones are CDN enabled.

While the second case is annoying, its not the worst. I do believe that the way cloudfiles handles this with the isCdn flag may be the best option. Usually if you want to deal with CDN containers, you would know so going into it. The explicit flag makes sense in this case.

@EdLeafe
Copy link

EdLeafe commented May 31, 2013

@kenperkins Essentially, yes. But that depends on some other things. If the service catalog returned from auth doesn't have an endpoint for 'object_cdn', then none of the CDN-specific stuff is ever done for any containers, and those container attributes remain in their initialized value of None. But if there is such an endpoint defined, when a container object (in Python parlance) is created, pyrax does a HEAD request for that container. If that comes back empty, the container is not CDN-enabled, and nothing else happens. But if it is, the response from the HEAD request contains all of the CDN info in the headers, so they get parsed into the cdn_* attributes of the container.

Clearer?

@kenperkins
Copy link
Member

Pull Request #161 added with the new implementation.

@kenperkins kenperkins closed this Jun 18, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants