Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1143)

Issue 10830044: Unify providers' state storage implementations. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 9 months ago by jtv.canonical
Modified:
10 years, 8 months ago
Reviewers:
mp+172373, jameinel, fwereade
Visibility:
Public.

Description

Unify providers' state storage implementations. I've been asked to unify the environs.Environ.StateInfo() implementations that are shared just about verbatim between the EC2, OpenStack MAAS, and now the Azure providers. But those build on bootstrapState, loadState, and saveState which are also all duplicated between those providers. And so here I unify those implementations. A provider is still free to implement its own alternative, and it would be easy enough to extend the signatures for LoadState and SaveState so that other providers could extend their own versions of BootstrapState but still use the same code. Once this is done, I can create a shared implementation of StateInfo() which will sit in the same file, and make use of LoadState/SaveState. Even more code can disappear. Looking at the code, you'll note that LoadState/SaveState no longer involve an environment. All they care about actually is an environs.Storage implementation, and with the decoupling, it becomes very easy to run the tests against the localstorage implementation. https://code.launchpad.net/~jtv/juju-core/shared-state/+merge/172373 (do not edit description out of merge proposal)

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+79 lines, -270 lines) Patch
A [revision details] View 1 chunk +2 lines, -0 lines 0 comments Download
M environs/azure/environ.go View 1 chunk +1 line, -1 line 0 comments Download
M environs/azure/environ_test.go View 3 chunks +15 lines, -2 lines 0 comments Download
D environs/azure/state.go View 1 chunk +0 lines, -53 lines 0 comments Download
M environs/ec2/ec2.go View 3 chunks +3 lines, -3 lines 0 comments Download
M environs/ec2/export_test.go View 1 chunk +0 lines, -12 lines 0 comments Download
M environs/ec2/local_test.go View 2 chunks +2 lines, -2 lines 0 comments Download
M environs/maas/environ.go View 2 chunks +4 lines, -2 lines 0 comments Download
M environs/maas/environ_test.go View 2 chunks +4 lines, -2 lines 0 comments Download
D environs/maas/state.go View 1 chunk +0 lines, -53 lines 0 comments Download
D environs/maas/state_test.go View 1 chunk +0 lines, -26 lines 0 comments Download
M environs/openstack/export_test.go View 2 chunks +0 lines, -13 lines 0 comments Download
M environs/openstack/local_test.go View 2 chunks +2 lines, -2 lines 0 comments Download
M environs/openstack/provider.go View 3 chunks +3 lines, -3 lines 0 comments Download
D environs/openstack/state.go View 1 chunk +0 lines, -47 lines 0 comments Download
M environs/state.go View 2 chunks +20 lines, -14 lines 0 comments Download
M environs/state_test.go View 2 chunks +22 lines, -34 lines 0 comments Download
M worker/deployer/simple.go View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 3
jtv.canonical
Please take a look.
10 years, 9 months ago (2013-07-01 17:29:07 UTC) #1
jameinel
Very nice to see this code merged together. LGTM
10 years, 9 months ago (2013-07-01 18:55:02 UTC) #2
fwereade
10 years, 9 months ago (2013-07-02 10:02:25 UTC) #3
Lovely, LGTM
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b