-
Notifications
You must be signed in to change notification settings - Fork 8
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
initial prototype #8
Conversation
o rocket request guards only return statuses and error catchers lack any context of what caused the error, so attempt to have all error handling go through handlers o failure's nice but doesn't tie in neatly w/ an API such as rocket's into_outcome
hack around into_outcome w/ Into::into
src/http.rs
Outdated
// REST Functions | ||
|
||
/// Set a version for a broadcaster / collection | ||
#[post("/v1/broadcasts/<_broadcaster_id>/<collection_id>", data = "<version>")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about suggesting the data be a structure like JSON, in case we need to add additional aspects (like distribution percentage, or scheduling) but then realized that's all v2
crap.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a bad idea. POST -> {"version": "v1"}
GET should then maybe return another level of indirection to match:
GET <- {"broadcasts": {"foo/bar": {"version": "v1"}}}
let broadcaster_id = request | ||
.get_param::<String>(0) | ||
.map_err(HandlerErrorKind::RocketError) | ||
.map_err(Into::into) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These Into::into calls are a hack to make our failure based HandlerErrorKinds/ Context<HandlerErrorKinds>
work with rocket's into_outcome.
Normally one would use the ? operator (try!) directly on these failure types. ? calls .into() on them automatically converting them to the canonical HandlerError (via the typical From impls pattern in our error module). The into_outcome API's in a tricky place here because it sits in between these types and the ultimate ? call and ends up not working on them (fails to compile)
I think it could potentially be changed to call .into() here for us making these unnecessary (and generally making it a better citizen w/ failure.. to be continued). into_outcome calls aren't required here either, but it's a nice API that should just work
src/db/schema.rs
Outdated
@@ -0,0 +1,6 @@ | |||
table! { | |||
versionv1 (service_id) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should probably versionv1 -> broadcastv1, service_id -> id
deref Conn into a diesel Connection
@@ -0,0 +1,5 @@ | |||
CREATE TABLE versionv1 ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe call the main table service_versions?
CREATE TABLE versionv1 ( | ||
service_id VARCHAR(200) NOT NULL, | ||
version VARCHAR(200) NOT NULL, | ||
PRIMARY KEY(service_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be nice to store a last_changed timestamp.
@@ -0,0 +1,5 @@ | |||
CREATE TABLE versionv1 ( | |||
service_id VARCHAR(200) NOT NULL, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably store it as two columns (broadcaster, version), otherwise we'll need to do string comparisons to locate everything by the same broadcaster, etc.
@@ -0,0 +1,7 @@ | |||
CREATE TABLE broadcastsv1 ( | |||
broadcaster_id VARCHAR(64) NOT NULL, | |||
bchannel_id VARCHAR(128) NOT NULL, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
collection_id? We call it the collection in our API spec, though maybe that's too Kinto specific. I do like how channel is more akin to broadcaster.
merge the initial prototype from wip:
o basic rocket setup w/ error handling
o initial db setup w/ diesel+mysql
o skeleton of needed auth calls
o some tests + travis