cache-server: provides client-related functionality for dealing with a shard on the HTTP level #1853
Conversation
|
/assign @stevekuznetsov @sttts |
p0lyn0mial
force-pushed
the
cache-server-client-rt
branch
from
c1a1ba6 to
9e12c04
Compare
p0lyn0mial
force-pushed
the
cache-server-client-rt
branch
from
9e12c04 to
04676f9
Compare
|
@stevekuznetsov I slightly changed the PR, PTAL |
p0lyn0mial
force-pushed
the
cache-server-client-rt
branch
from
04676f9 to
5921e93
Compare
pkg/cache/client/context.go
Outdated
| // ShardFromContext returns the value of the shard key on the ctx, | ||
| // or an empty Name if there is no shard key. | ||
| func ShardFromContext(ctx context.Context) shard.Name { | ||
| val, ok := ctx.Value(shardContextKey).(shard.Name) |
There was a problem hiding this comment.
nit: can be a follow-up: can just return ctx.Value(shardContextKey).(shard.Name) since
a) the shardContextKey is private to this package, so the only case where !ok is if someone were to write code in this package to add something invalid
b) the zero-value is already shard.New("")
There was a problem hiding this comment.
yes, I just don't like when the code panics but sure I can change it.
pkg/cache/client/round_tripper.go
Outdated
| return originalPath, nil | ||
| } | ||
| // if the originalPath already has a shard set just overwrite it to the given one | ||
| if strings.Contains(originalPath, "/shards") { |
There was a problem hiding this comment.
Is this specifically not doing a prefix check for some reason?
There was a problem hiding this comment.
as of today, we could actually check for a prefix, maybe in the future, it will have to be changed.
openshift-ci
bot
added
do-not-merge/hold
… HTTP level WithDefaultShardRoundTripper for setting a default shard in the context. WithShardRoundTripper for changing the URL path to contain a shard name defined in the context. WithShardInContext and ShardFromContext for writting/reading a shard name to/from the context. A shard package for holding a shard name in the memory.
p0lyn0mial
force-pushed
the
cache-server-client-rt
branch
from
5921e93 to
43e9e48
Compare
|
@stevekuznetsov updated the PR, PTAL /hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: stevekuznetsov The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Summary
WithDefaultShardRoundTripperfor setting a default shard in the context.WithShardRoundTripperfor changing the URL path to contain a shard name defined in the context.WithShardInContextandShardFromContextfor writting/reading a shard name to/from the context.A shard package for holding a shard name in the memory.
Related issue(s)
Fixes #