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

implementation of store that uses http protocol #241

Merged
merged 4 commits into from
Oct 21, 2014

Conversation

koertkuipers
Copy link
Contributor

No description provided.

def apply(dest: String): HttpStringStore = new HttpStringStore(Http.newService(dest))
}

class HttpStringStore(val client: Service[HttpRequest, HttpResponse]) extends Store[String, String] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this one can be implemented as a ConvertedStore?
if you have an Injection[String, ChannelBuffer]

class HttpStringStore(store: HttpStore)
  extends ConvertedStore[String, String, ChannelBuffer, String](store)(identity)(inj)

@rubanm
Copy link
Contributor

rubanm commented Oct 17, 2014

@koertkuipers Thanks for the PR! One comment on using ConvertedStore. LGTM otherwise.

@koertkuipers
Copy link
Contributor Author

failure seems unrelated to pullreq. want to to try get travis going again?

@rubanm
Copy link
Contributor

rubanm commented Oct 20, 2014

Restarted ..

@rubanm
Copy link
Contributor

rubanm commented Oct 20, 2014

@koertkuipers Looks like the http store tests never ran. Want to add an entry here?
https://github.com/twitter/storehaus/pull/241/files#diff-215113124f1de02f228327ba7abb45f7R135

@koertkuipers
Copy link
Contributor Author

ah my bad. i will add it

ianoc added a commit that referenced this pull request Oct 21, 2014
implementation of store that uses http protocol
@ianoc ianoc merged commit 7cb17e5 into twitter:develop Oct 21, 2014
@ianoc
Copy link
Collaborator

ianoc commented Oct 21, 2014

Tests look good, merged

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.

3 participants