-
Notifications
You must be signed in to change notification settings - Fork 29
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
xds client implementation #46
Conversation
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
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'm excited about this! A few questions to begin with:
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
Last set of comments, otherwise looks LGTM. will approve once that select statement is cleaned up. |
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
defer cancelFunc() | ||
select { | ||
case <-ctx.Done(): | ||
// Context was cancelled, hence this is not an erroneous scenario. |
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.
Shouldn't this be above the case? 🤔 Not sure about go formatting so I could be wrong!
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.
hm...i will look at why lint is not catching it in the next PR.
* Don't extend TTL for AddWatch (envoyproxy#60) Fixes envoyproxy#58 The expiration time should only be delayed in SetResponse, since the intent of the TTL was to avoid stale responses in the cache. Adding/removing watches should not affect the expiration time of a key. This change removes that extension from AddWatch, and envoyproxy#59 already omits the expiry time extension. Signed-off-by: Lisa Lu <lisalu@lyft.com> * xds client implementation (envoyproxy#46) Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com> * add With Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com> Co-authored-by: Lisa Lu <lisalu@lyft.com>
Signed-off-by: Jyoti Mahapatra jmahapatra@lyft.com