Rename chunk.ObjectClient and implement generic object store chunk client #2108
Conversation
jtlisi
changed the title
pkg/chunk/testutils/testutils.go
Outdated
| @@ -22,7 +23,7 @@ const ( | |||
| // Fixture type for per-backend testing. | |||
| type Fixture interface { | |||
| Name() string | |||
| Clients() (chunk.IndexClient, chunk.ObjectClient, chunk.TableClient, chunk.SchemaConfig, error) | |||
| Clients() (chunk.IndexClient, chunk.Client, chunk.ObjectClient, chunk.TableClient, chunk.SchemaConfig, error) | |||
There was a problem hiding this comment.
Having both chunk.Client and chunk.ObjectClient sounds redundant to me. Could you explain why we need chunk.ObjectClient too?
There was a problem hiding this comment.
I can have the downstream fixtures wrap the object client before returning them
jtlisi
force-pushed
the
20200210_rename_chunk_objectclient
branch
2 times, most recently
from
5179c15 to
34315dd
Compare
| @@ -155,7 +155,7 @@ func NewDynamoDBIndexClient(cfg DynamoDBConfig, schemaCfg chunk.SchemaConfig) (c | |||
| } | |||
|
|
|||
| // NewDynamoDBObjectClient makes a new DynamoDB-backed ObjectClient. | |||
There was a problem hiding this comment.
ObjectClient > chunk.Client
There was a problem hiding this comment.
Will do, I will also rename this function to be NewDynamoDBChunkClient
pkg/chunk/storage/factory.go
Outdated
| @@ -164,13 +165,12 @@ func NewIndexClient(name string, cfg Config, schemaCfg chunk.SchemaConfig) (chun | |||
| } | |||
|
|
|||
| // NewObjectClient makes a new ObjectClient of the desired types. | |||
There was a problem hiding this comment.
ObjectClient > chunk.Client
pkg/chunk/storage/factory.go
Outdated
| @@ -164,13 +165,12 @@ func NewIndexClient(name string, cfg Config, schemaCfg chunk.SchemaConfig) (chun | |||
| } | |||
|
|
|||
| // NewObjectClient makes a new ObjectClient of the desired types. | |||
| func NewObjectClient(name string, cfg Config, schemaCfg chunk.SchemaConfig) (chunk.ObjectClient, error) { | |||
| func NewObjectClient(name string, cfg Config, schemaCfg chunk.SchemaConfig) (chunk.Client, error) { | |||
There was a problem hiding this comment.
I have to admit that now the function name NewObjectClient() is quite confusing, cause it returns chunk.Client and not chunk.ObjectClient. Given it's just called in this file, we may easily rename it to NewChunkClient.
There was a problem hiding this comment.
I'll update it to reflect the changes.
…eneric ObjectClient chunk.Client Signed-off-by: Jacob Lisi <jacob.t.lisi@gmail.com>
Signed-off-by: Jacob Lisi <jacob.t.lisi@gmail.com>
Signed-off-by: Jacob Lisi <jacob.t.lisi@gmail.com>
Signed-off-by: Jacob Lisi <jacob.t.lisi@gmail.com>
Signed-off-by: Jacob Lisi <jacob.t.lisi@gmail.com>
jtlisi
force-pushed
the
20200210_rename_chunk_objectclient
branch
from
f9238f9 to
e48f0af
Compare
What this PR does:
ObjectClienttoClientpkg/chunk/objectobject.ChunkClientwhen S3/GCS/AzureBlobStore/LocalFS is specified as a chunk store, all of these backends already implement theObjectClientinterface.Which issue(s) this PR fixes:
Fixes #2107
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]