-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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: implement RouteAction timeout support #4116
Conversation
} | ||
|
||
func (r route) String() string { | ||
return r.m.String() + "->" + fmt.Sprint(r.action) | ||
return r.m.String() + "->" + fmt.Sprint(r.clusters) |
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.
Also print the duration? Will be hard to format though.
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.
Done. I thought about this earlier but forgot to do it.
I think the formatting looks good:
pathPrefix:/foo -> { clusters: [{A 1}], maxStreamDuration: 5s }
pathPrefix: -> { clusters: [{cluster_1 75} {cluster_2 25}], maxStreamDuration: 0s }
I needed to go into the WRRs to add String
methods to clean this up (I broke the formatting in my previous PR since it was a map and became a WRR instance).
} | ||
|
||
func (r route) String() string { | ||
return r.m.String() + "->" + fmt.Sprint(r.action) | ||
return fmt.Sprintf("%s -> { clusters: %v, maxStreamDuration: %v }", r.m.String(), r.clusters, r.maxStreamDuration) |
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.
Add the String
method to the wrr.WRR
interface.
Actually, why does this work since String
is not part of the interface?
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.
The interface is fmt.Stringer
. I don't think we need to explicitly add Stringer
everywhere we want to print things; it's a magic part of standard library stuff.
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.
Yeah, sorry. I thought you were calling String()
on clusters
.
} | ||
|
||
func (r route) String() string { | ||
return r.m.String() + "->" + fmt.Sprint(r.action) | ||
return fmt.Sprintf("%s -> { clusters: %v, maxStreamDuration: %v }", r.m.String(), r.clusters, r.maxStreamDuration) |
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.
Yeah, sorry. I thought you were calling String()
on clusters
.
This is missing one last piece from A31: reading the HTTP connection manager default max_stream_duration. I'll add that in a follow-on PR.