storage: Split pkg cloud into cloud interface and cloud impl #50644
Conversation
adityamaru
requested review from
a team and
pbardea
and removed request for
a team
|
This change is |
adityamaru
requested review from
dt and
miretskiy
and removed request for
pbardea
|
I will rebase once #50493 is merged. I just wanted to get a CI run in to make sure I haven't botched up the move. |
adityamaru
force-pushed
the
split-interface-concrete-cloud
branch
from
391b52a to
26aa14e
Compare
There was a problem hiding this comment.
Reviewed 38 of 38 files at r2.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @adityamaru and @dt)
pkg/storage/cloud/external_storage.go, line 19 at r2 (raw file):
// ExternalStorage provides functions t
You should update the comment "ExternalStorage provides API..."
Also make it very clear that this file should not have any implementation code -- it's for interfaces only.
adityamaru
force-pushed
the
split-interface-concrete-cloud
branch
from
26aa14e to
35f3ac7
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/storage/cloud/external_storage.go, line 19 at r2 (raw file):
Previously, miretskiy (Yevgeniy Miretskiy) wrote…
// ExternalStorage provides functions tYou should update the comment "ExternalStorage provides API..."
Also make it very clear that this file should not have any implementation code -- it's for interfaces only.
done.
pkg/testutils/lint/lint_test.go, line 441 at r4 (raw file):
cloudimpl
@miretskiy fyi I had to update the lint allow list for using os.getEnv
adityamaru
force-pushed
the
split-interface-concrete-cloud
branch
from
35f3ac7 to
6df4be3
Compare
Previously, both the ExternalStorage interface and all the specialized implementations were in `pkg/storage/cloud`. This PR moves the interface and factory method signatures to `pkg/storage/cloud` and all the implementations to `pkg/storage/cloudimpl`. The motivation behind this is to ensure that other packages such `pkg/sql` and `pkg/kv` depend on just the interface/factory signatures (not the concrete impl) to prevent dependency cycles. More concreteley, we need to plumb an InternalExecutor and kv.DB for our new user scoped file-table storage. This was previously not possible because of a `pkg/sql -> pkg/storage/cloud ->pkg/sql` cycle. Now, since the executor will be used in `cloudimpl` this cycle will not occur. Release note: None
adityamaru
force-pushed
the
split-interface-concrete-cloud
branch
from
6df4be3 to
1eaeed8
Compare
|
TFTR! bors r=miretskiy |
Build succeeded |
Previously, both the ExternalStorage interface and all the specialized
implementations were in
pkg/storage/cloud.This PR moves the interface and factory method signatures to
pkg/storage/cloudand all the implementations topkg/storage/cloudimpl. The motivation behind this is to ensure thatother packages such
pkg/sqlandpkg/kvdepend on just theinterface/factory signatures (not the concrete impl) to prevent
dependency cycles.
More concreteley, we need to plumb an InternalExecutor and kv.DB for our
new user scoped file-table storage. This was previously not possible
because of a
pkg/sql -> pkg/storage/cloud ->pkg/sqlcycle. Now, sincethe executor will be used in
cloudimplthis cycle will not occur.Informs: #47211
Release note: None