unwrap client #130
unwrap client #130
Conversation
Signed-off-by: Bulat Shakirzyanov <83289+avalanche123@users.noreply.github.com>
avalanche123
force-pushed
the
unwrap-client
branch
from
11bb3e9
to
c228f65
Compare
| @@ -117,6 +116,7 @@ func Anonymize(cfg *rest.Config) *rest.Config { | |||
| // type the client is asked to get or list. Clients (and their caches) expire | |||
| // and are garbage collected if they are unused for five minutes. | |||
| type Cache struct { | |||
| ctx context.Context | |||
There was a problem hiding this comment.
instead of creating an adhoc context when initializing a client, create a context that will be valid for the lifetime of Cache
| "new-expiry", time.Now().Add(c.expiry), | ||
| ) | ||
| sn.expiration.Reset(c.expiry) | ||
| return sn.client, nil |
There was a problem hiding this comment.
return session client directly, client expiration is extended when a client is requested, rather than used. This simplifies code as we don't have to wrap the client anymore and still preserves cached clients since their expiration is extended from each resolver that needs to use them. extending client's expiration during operations would only be useful if the expiration duration can be lower than a duration if any one client operation, which a very unlikely situation.
| @@ -295,15 +299,18 @@ func (c *Cache) Get(cr auth.Credentials, o ...GetOption) (client.Client, error) | |||
| // is possible to 'reset' (i.e. extend) a ticker. | |||
| expiration := &tickerExpiration{t: time.NewTicker(c.expiry)} | |||
| newExpiry := time.Now().Add(c.expiry) | |||
| ctx, cancel := context.WithCancel(context.Background()) | |||
| sn = &session{client: wc, cancel: cancel, expiry: c.expiry, expiration: expiration, log: log} | |||
| ctx, cancel := context.WithCancel(c.ctx) | |||
There was a problem hiding this comment.
use Cache's context for constructing the client instead of creating a new context.
| } | ||
| c.remove(id) |
There was a problem hiding this comment.
remove is idempotent, so let it run no matter how we got here.
Description of your changes
Simplify client - extend expiry on get, and don't wrap client in session.
I have:
make reviewableto ensure this PR is ready for review.Addedbackport release-x.ylabels to auto-backport this PR, as appropriate.How has this code been tested
Tested locally.