Add useragent #33
Add useragent #33
Conversation
pkg/client/useragent.go
Outdated
| return clientWithAgent | ||
| } | ||
|
|
||
| func (s *sharedClientFactoryWithAgent) clientWithAgent(client *Client) (*Client, error) { |
There was a problem hiding this comment.
Why not use client.WithAgent?
There was a problem hiding this comment.
🤔 good call, WithAgent properly creates a copy as well. Replaced.
| Debugf = func(message string, obj ...interface{}) { | ||
| log.Printf("DEBUG: "+message+"\n", obj...) | ||
| } |
There was a problem hiding this comment.
nit: Shouldn't these just be functions instead of variables pointing to functions?
| Debugf = func(message string, obj ...interface{}) { | |
| log.Printf("DEBUG: "+message+"\n", obj...) | |
| } | |
| ) | |
| func Debugf(message string, obj ...interface{}) { | |
| log.Printf("DEBUG: "+message+"\n", obj...) | |
| } |
There was a problem hiding this comment.
I did not write the package and am not sure what the intention was. For now I am conforming to how the other logs forms are written but for the longer term I made this issue for replacing it with an external logging package: rancher/wrangler#244.
pkg/client/client.go
Outdated
| if config == nil { | ||
| return | ||
| } |
There was a problem hiding this comment.
I'm curious why you put the config == nil check here rather than around the caller. If someone uses setConfig elsewhere a nil config would be silently skipped. Where it's called this appears to be done for backwards compatibility. Shouldn't the nil check be there for better understanding of reading a section of code? Just thinking about it as someone who would read it 6 months from now and not be any of the reviewers or author.
There was a problem hiding this comment.
I totally agree with this. There is no reason why it wouldn't be passed by value otherwise as well so I changed the function to accept it as a value.
rmweir
force-pushed
the
add-useragent
branch
2 times, most recently
from
e8f7d20 to
5ce15a5
Compare
rmweir
force-pushed
the
add-useragent
branch
2 times, most recently
from
f376448 to
0aeab37
Compare
rmweir
force-pushed
the
add-useragent
branch
from
0aeab37 to
c40248c
Compare
|
@cmurphy do you want to re-review due to changes? |
pkg/cache/cache.go
Outdated
| @@ -1,3 +1,6 @@ | |||
| /* | |||
| Package cache provides a cache that maintains a list for a resource type which it updates as it | |||
There was a problem hiding this comment.
as it ...what?
sorry, missed this before
There was a problem hiding this comment.
@rmweir you've left us hanging in suspense!
There was a problem hiding this comment.
I think I was mid comment when I realized I didn't end up making any changes to the cache package 😛 . Removed.
Added the ability to configure a client's user agent. Added wrappers for controllers, and factories that provide the ability to configure the user agent of the clients they use.
rmweir
force-pushed
the
add-useragent
branch
from
c40248c to
b7ec3b3
Compare
Add the ability to configure useragent
Added the ability to configure a client's user agent. Added wrappers
for controllers, and factories that provide the ability to configure
the user agent of the clients they use.
Issue:
rancher/rancher#38775