-
Notifications
You must be signed in to change notification settings - Fork 7
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
Centralized resources #74
Conversation
add resources clear method add watchers for resources
this allows global options to be applied to page config resources
rename resources config
It'd be cleaner if the object here was more similar to those defined per view in #73 and maybe even {
"http://staging.proxy.storyteller.io/Fauntleroy": {
"resources": {
"profile": "/soundcloud/users/{username}.json",
"favorites": "/soundcloud/users/{username}/favorites.json"
},
"headers": {
"Api-Key": "e801cf0a-ee44-42d3-a29f-aff82fd0521b"
}
},
"http://services.sparkart.net/api/v1/consumer": {
"resources": {
"events": "/events?tags={username}"
},
"qs": {
"key": "551fca3b-a134-47a3-a34a-fea6ac38af12"
}
}
} The same format makes sense to me within a view. I think defining headers/auth per resource could get way too dense and redundant. What if I needed to specify the same header for 5+ resources? Not sure if you included support for query strings in #73 but you should. I'd prefer to spell it out as "querystring" but I figured you're trying to maintain parity with Request. Does this method replace #73? Or are you supporting both methods for backwards compatibility (and optionality?). If both are supported concurrently, how do you manage name conflicts? "Local"-defined resources in a view win? |
One more thing: we should really have the ability to source these credentials from environment variables. That will allow us to template the configuration, keep repos public, and generally improve security. Would you prefer another issue for that? |
I'm not sure what you mean by this comment:
If you want to do that, you specify the headers in the {
"options": {
"http://staging.proxy.storyteller.io/*": {
"headers": {
"Api-Key": "e801cf0a-ee44-42d3-a29f-aff82fd0521b"
}
}
}
} Now any resources that match the There is no "support" for querystrings aside from including them in the URL proper. Right now options are passed directly to Hyperquest as is, and there is no way to configure query strings aside from directly setting them in the URL. This should be easy enough to support, but I'll have to handle it myself. This doesn't replace in-view resource definitions, it just augments them. Resources defined at this level can be used within pages by name. It's a little bit redundant, but if you define a resource named
|
Sourcing credentials from the ENV is a wholly separate issue. |
It was definitely unclear how this file relates to per-view definition. What about the array of names that was discussed before? If I want to reference these global resources do I need to do this name association in every view? What's the value of per-view definition with the introduction of this file?
Does that not mean that it inherits request's API, which includes a |
@pushred took some time to see if |
Thanks for checking. Just limits the utility of the options outside of Storyteller.io (or other APIs that use headers/basic auth). Did the alternative object hierarchy/resource URIs (vs URLs) do anything for you? |
I'm not too keen on that hierarchy because it removes the 1-to-1 association of the |
Well again, what is the value of per-view resource definition with the addition of centralized resources? If there is value then I think there's already a disconnect because of configuration keys like {
"context": {
"title": "Superband",
"description": "My favorite music, upcoming events, and more!",
"og:image": resources.profile.avatar_url,
"http://proxy.storyteller.io/Fauntleroy": {
"resources": {
"profile": "/soundcloud/users/{username}.json",
"favorites": "/soundcloud/users/{username}/favorites.json"
},
"headers": {
"Api-Key": "e801cf0a-ee44-42d3-a29f-aff82fd0521b"
}
},
"http://services.sparkart.net/api/v1/consumer": {
"resources": {
"events": "/events?tags={username}"
},
"qs": {
"key": "551fca3b-a134-47a3-a34a-fea6ac38af12"
}
}
},
"layout": "/layouts/3-column",
"preprocessor": "superband"
} This adds a layer of nesting but explicitly calling out Maybe {
"context": {
"title": "Superband",
"description": "My favorite music, upcoming events, and more!",
"og:image": resources.profile.avatar_url,
"http://proxy.storyteller.io/Fauntleroy": {
"profile": "/soundcloud/users/{username}.json",
"favorites": "/soundcloud/users/{username}/favorites.json"
},
"http://services.sparkart.net/api/v1/consumer": {
"events": "/events?tags={username}"
}
},
"layout": "/layouts/3-column",
"preprocessor": "superband"
}
[{
"hostname": "http://proxy.storyteller.io",
"headers": {
"Api-Key": "e801cf0a-ee44-42d3-a29f-aff82fd0521b"
}
}, {
"hostname": "http://services.sparkart.net",
"querystring": {
"key": "551fca3b-a134-47a3-a34a-fea6ac38af12"
}
}, {
"hostname": "https://basecamp.com",
"basic": {
"username": "elanehart",
"password": "temp123"
}
}] |
@pushred I've added the ability to set query params like so: "test8": {
"url": "https://hipster.sparkart.net/api/v1/resources/poiu/my-resource-8",
"query": {
"test": true
}
},
"test9": {
"url": "https://hipster.sparkart.net/api/v1/resources/poiu/my-resource-9",
"query": {
"test": "{resource_test}"
}
} |
Cool, that should make reading param configs easier too. |
Also, re: "why even have per page resource config when we have a centralized store?": That's mostly because of backwards compatibility. I don't really think there's any reason to keep those config options there otherwise, not off the top of my head at least. |
While I get what you're going for, I think this format would be more confusing for end users: {
"context": {
"title": "Superband",
"description": "My favorite music, upcoming events, and more!",
"og:image": resources.profile.avatar_url,
"http://proxy.storyteller.io/Fauntleroy": {
"profile": "/soundcloud/users/{username}.json",
"favorites": "/soundcloud/users/{username}/favorites.json"
},
"http://services.sparkart.net/api/v1/consumer": {
"events": "/events?tags={username}"
}
},
"layout": "/layouts/3-column",
"preprocessor": "superband"
} We can ask the team about it, but I never liked putting URLs into json keys. I'm amenable to creating something like a
|
I was thinking that was the reason, doesn't really seem worth keeping this functionality around though only for that. It's what major versions are for, and updating our existing sites wouldn't require that much effort. Now's really the time to make this sort of change. But let's see what @monikahoex and @krackydile have to say, maybe there's some value that we're overlooking. Aren't you putting URLs in keys for the options object? |
I am using URLs with wildcards in the |
How about explicitly associating an auth with a resource: https://gist.github.com/Fauntleroy/7593588 |
Pretty repetitive still, I think it begs the question of the use case of using different credentials for different (or the same endpoint) within the same API. I can think of one case where we came close: on Young Money the original plan was to have two Tumblr accounts: one for Young Money and one for Lil Wayne. If we'd done that we may have needed separate apps and credentials in order to access drafts for each respective account. But even maintaining 2 separate Tumblr blogs was too much for them in that case so didn't happen. But since it's actually the access token granted based on the account providing auth so long as the account had access to both blogs we wouldn't have needed separate credentials. Specifying base URLs would still accommodate similar scenarios though if it could include parts of path segments in addition to the host. |
I'm partial to using named auth keys over some kind of URL matching scheme. It makes it more explicit to end users and it basically removes the chance of there being any weird edge cases. I'm not sure about the |
Sorry sorry, I haven't had time to read through this all yet - I've been in Productionville. Do you need an answer today? @krackydile and I spoke with @Fauntleroy last night and the concept makes sense to us, but I haven't had time to review the entire conversation here, yet. |
I'm not into the idea of having a separate "keychain" file where we maintain all of the authentication keys, etc. I think it breaks an already shaky mental model of how resources work, and once we add in dynamic segments and pagination, it's going to get even more unmanageable (mentally). I think removing the resources at the view level is going to take another mental leap to understand dynamic segments and the pages they're used on. Of the options in this pull request, I think this is the easiest to comprehend. {
"http://proxy.storyteller.io/Fauntleroy": {
"resources": {
"profile": "/soundcloud/users/{username}.json",
"favorites": "/soundcloud/users/{username}/favorites.json"
},
"headers": {
"Api-Key": "e801cf0a-ee44-42d3-a29f-aff82fd0521b"
}
},
"http://services.sparkart.net/api/v1/consumer": {
"resources": {
"events": "/events?tags={username}"
},
"qs": {
"key": "551fca3b-a134-47a3-a34a-fea6ac38af12"
}
}
} I understand that for us, most if, not all, of our resources are going to come through Storyteller.io. I think this would be easily extensible to other users who might not be using Storyteller.io. |
Ideally we could support both methods, to allow for overrides and as a preference for developers like Monika who want to cut down on the framework automagic that can lead to confusion. Personally I think credentials all over the place is duplication and begs for a configuration file, but I'm probably an "advanced" user. If the credentials ever change you're also going to have to search/replace all over to make the updates. #75 at least would address the worst of that, and keep sensitive credentials committed to dozens of files out of version control.
Yeah I was pretty confused about this initially too actually, I kept asking "so how are dynamic segments gonna work?" without realizing that they worked all along. I had some weird inclination towards wanting to pass their values as arguments or something when adding resources to a page. But of course the resource URLs are just considered in the context of the page/route they are added to. Still, weird nonetheless. The original push for this was #43 and #48 which are both concerned with requesting resources for use clientside. I agree with this sentiment from #48:
I still think there is value in being able to leverage the serverside preprocessors here. But we can experiment with going clientside all the way and come back to that issue if we find that it is actually an issue/feature needed. I suggest using the Universe API to begin with as we already support CORS there, and as I've mentioned I think we should explore a better alternative to the Universe widgets than https://github.com/sparkartgroupinc/sparkart.js leveraging some of the new tools at our disposal, such as the helpers and the potential sharing of preprocessor modules server/clientside using Browserify. |
I don't find this format confusing. I think grouping the options by the different resource endpoint URLs makes sense to me. |
Would centralized resources allow us to improve the SEO of pages by allowing us to have dynamic |
Not specifically, I alluded to another idea to solve that in the format example linked above though: the ability to specify a key within a resource for the value of a key (#46) in the context. I'll open a separate issue about that later today. |
If we do something like this format we'll still have resource URLs coded in every view. While that's fine, I don't really see any reason to change the way resources are defined in views (since we already have resource URLs in every view). Resources are rarely changed, so I feel like the convenience of not having to retype What we could do is just keep the current resource definitions in views, and then specify authentication information in |
It's less about retyping and more about focusing attention on the endpoint. Same reason most API consoles and documentation hide the base URLs. You could consider that a separate feature for the future though (as an optional wrapper / relative resource URL definition) and retain the traditional 1:1 format here. Does [{
"http://proxy.storyteller.io/*": {
"headers": {
"Api-Key": "e801cf0a-ee44-42d3-a29f-aff82fd0521b"
}
}
}, {
"http://services.sparkart.net/*": {
"qs": {
"key": "551fca3b-a134-47a3-a34a-fea6ac38af12"
}
}
}] I'd vote for expanding Anyway seems like we should just close this as we're really going back to #73 and moving the options object into |
{
"http://proxy.storyteller.io/*": {
"headers": {
"Api-Key": "e801cf0a-ee44-42d3-a29f-aff82fd0521b"
}
},
"http://services.sparkart.net/*": {
"query": {
"key": "551fca3b-a134-47a3-a34a-fea6ac38af12"
}
}
} |
Lets developers define resources "globally" with a
resources.json
file. This file has 2 keys:resources
andoptions
.resources
is an object of resource data that operates exactly like the resources definition inside page configurations.options
is an object of resource options that are mixed into existing resource definitions (inresources.json
or page configurations). The keys here are strings that matchurl
s with a wildcard (*
), and the values are the options which you wish to set (right now that's justheaders
andauth
).Here's the
resources.json
I worked with in my example site:I'm not really a huge fan of how this has ended up, but it works. I'd love some suggestions, or maybe a different approach altogether...