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

Extract and test protobuf-over-http code #106

Closed
klingerf opened this issue Jan 4, 2018 · 3 comments · Fixed by #177
Closed

Extract and test protobuf-over-http code #106

klingerf opened this issue Jan 4, 2018 · 3 comments · Fixed by #177
Assignees

Comments

@klingerf
Copy link
Member

klingerf commented Jan 4, 2018

As part of #103, our client/server implementations for the public API were refactored, but they're still pretty convoluted. Specifically, the protobuf-over-http setup is brittle and not thoroughly tested. We should move that code to a separate package and add additional tests.

@klingerf klingerf self-assigned this Jan 4, 2018
@pcalcado
Copy link
Contributor

pcalcado commented Jan 4, 2018

I'm not sure if possible and/or should be part of the scope here but I'd really ❤️ to see test cases that actually run a client against a server on the same process and localhost and exercises some endpoints to end-to-end test the whole implementation.

@briansmith
Copy link
Contributor

I think what we're doing is duplicating stuff that's already a reusable part of client-go, so an alternative to this would be to reformulate our APIs to be implemented in terms of client-go's reusable library. If it can be made to work like that then I think that would be better than maintaining our own similar-but-different logic. I looked into this before and it seemed like it should be possible, but I admit I didn't quite understand the client-go code well enough to write a PR for it.

@klingerf
Copy link
Member Author

klingerf commented Jan 9, 2018

Am updating our client-go dependency in #122.

pcalcado added a commit that referenced this issue Jan 25, 2018
Previously, running `$conduit tap` would return a `Unexpected EOF` error when the server wasn't available. This was due to a few problems with the way we were handling errors all the way down the tap server. This change fixes that and cleans some of the protobuf-over-HTTP code.

- first step towards #49
- closes #106
khappucino pushed a commit to Nordstrom/linkerd2 that referenced this issue Mar 5, 2019
The `proxy::http::balance` module uses the `proxy::resolve::Resolve`
trait to implement a `Discover`.

This coupling between the balance and resolve modules prevents
integrating the destination profile API such that there is a per-route,
per-endpoint stack.

This change makes the `balance` stack generic over a stack that produces
a `Discover`. The `resolve` module now implements a stack that produces
a `Discover` and is generic over a per-endpoint stack.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants