This is much simpler than it looks. Motivation is as follows:
1) I'm trying to make our tools logic consistent, so we have some sort
of
chance of different parts of the system acting in concert (bootstrap,
start-instance, sync-tools, upgrade-juju, juju upgrades, ...?). It is
currently very difficult to build a mental model of how these things
are
related, and a common vocabulary will help.
2) I'd be crazy to try to do everything at once (ha!). So I'll leave
the
existing tools *selection* logic (FindTools, BestTools) in place while
I focus on just uploads/downloads.
3) Move tool-storagey code from environs to environs/tools:
* environs.listTools -> tools.ReadList
(environs.ListTools now uses tools.ReadList)
* environs.PutTools -> tools.Upload
(lots of code moved to storage.go and build.go in tools/)
* environs.ToolsStoragePath -> tools.StorageName
(it's a name, not a path, according to the storage metaphor)
...hmm. Lots of renames.
4) But wait, I can't do this, I need to use environs.Storage, and
there's
an import loop. To keep the diff small, define tools.URLLister and
tools.URLPutter.
6) The tests are starting to look somewhat repetitive, and I fear
subtle
differences creeping in. Let's bulk up environs/testing a little.
Ah, that's better.
From a review perspective, this branch is largely mechanical. The only
code
that is new, as opposed to moved or mechanically replaced, is:
* the tests for tools.ReadList (made explicit now it's exported).
* the tests for environs.EmptyStorage (yes, it's trivial. yes, it
should
still be tested.)
* environs/testing/tools.go (mostly new, and used in several places).
* UpgradeJujuCommand (surprising behaviour corrected, clarifying test
added).
Affected files:
A [revision details]
M cmd/juju/bootstrap.go
M cmd/juju/main_test.go
M cmd/juju/sync_tools.go
M cmd/juju/sync_tools_test.go
M cmd/juju/upgradejuju.go
M cmd/juju/upgradejuju_test.go
M cmd/jujud/agent_test.go
M cmd/jujud/upgrade_test.go
M environs/dummy/environs.go
M environs/ec2/live_test.go
M environs/ec2/local_test.go
M environs/export_test.go
M environs/jujutest/livetests.go
M environs/openstack/live_test.go
M environs/openstack/local_test.go
A environs/storage.go
A environs/storage_test.go
M environs/testing/tools.go
M environs/tools.go
A environs/tools/build.go
A environs/tools/export_test.go
M environs/tools/list.go
A environs/tools/storage.go
A environs/tools/storage_test.go
M environs/tools_test.go
Reviewers: mp+157770_ code.launchpad. net,
Message:
Please take a look.
Description:
environs: extract tools package
This is much simpler than it looks. Motivation is as follows:
1) I'm trying to make our tools logic consistent, so we have some sort
of
chance of different parts of the system acting in concert (bootstrap,
start-instance, sync-tools, upgrade-juju, juju upgrades, ...?). It is
currently very difficult to build a mental model of how these things
are
related, and a common vocabulary will help.
2) I'd be crazy to try to do everything at once (ha!). So I'll leave
the
existing tools *selection* logic (FindTools, BestTools) in place while
I focus on just uploads/downloads.
3) Move tool-storagey code from environs to environs/tools:
* environs.listTools -> tools.ReadList ListTools now uses tools.ReadList) ToolsStoragePat h -> tools.StorageName
(environs.
* environs.PutTools -> tools.Upload
(lots of code moved to storage.go and build.go in tools/)
* environs.
(it's a name, not a path, according to the storage metaphor)
...hmm. Lots of renames.
4) But wait, I can't do this, I need to use environs.Storage, and
there's
an import loop. To keep the diff small, define tools.URLLister and
tools.URLPutter.
6) The tests are starting to look somewhat repetitive, and I fear
subtle
differences creeping in. Let's bulk up environs/testing a little.
Ah, that's better.
From a review perspective, this branch is largely mechanical. The only
code
that is new, as opposed to moved or mechanically replaced, is:
* the tests for tools.ReadList (made explicit now it's exported). EmptyStorage (yes, it's trivial. yes, it testing/ tools.go (mostly new, and used in several places).
* the tests for environs.
should
still be tested.)
* environs/
* UpgradeJujuCommand (surprising behaviour corrected, clarifying test
added).
https:/ /code.launchpad .net/~fwereade/ juju-core/ environs- tools-storage/ +merge/ 157770
Requires: /code.launchpad .net/~fwereade/ juju-core/ environs- tools-list/ +merge/ 157964
https:/
(do not edit description out of merge proposal)
Please review this at https:/ /codereview. appspot. com/8545043/
Affected files: bootstrap. go main_test. go sync_tools. go sync_tools_ test.go upgradejuju. go upgradejuju_ test.go agent_test. go upgrade_ test.go dummy/environs. go ec2/live_ test.go ec2/local_ test.go export_ test.go jujutest/ livetests. go openstack/ live_test. go openstack/ local_test. go storage_ test.go testing/ tools.go tools/build. go tools/export_ test.go tools/list. go tools/storage. go tools/storage_ test.go tools_test. go
A [revision details]
M cmd/juju/
M cmd/juju/
M cmd/juju/
M cmd/juju/
M cmd/juju/
M cmd/juju/
M cmd/jujud/
M cmd/jujud/
M environs/
M environs/
M environs/
M environs/
M environs/
M environs/
M environs/
A environs/storage.go
A environs/
M environs/
M environs/tools.go
A environs/
A environs/
M environs/
A environs/
A environs/
M environs/