Issue 87760055: Use instance-type constraint

Issue 87760055: Use instance-type constraint (Closed)

Can't Edit
Can't Publish+Mail
Start Review

Created:
10 years ago by wallyworld

Modified:
9 years, 12 months ago

Reviewers:
mp+216256, fwereade

Visibility:
Public.

Description

Use instance-type constraint This branch now uses the instance-type constraint when searching for images. https://code.launchpad.net/~wallyworld/juju-core/image-search-uses-instance-type-constraint/+merge/216256 Requires: https://code.launchpad.net/~wallyworld/juju-core/instance-type-constraint/+merge/216244 (do not edit description out of merge proposal)

Patch Set 1 #

Patch Set 2 : Use instance-type constraint #

Total comments: 4

Patch Set 3 : Use instance-type constraint #

Created: 10 years ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+287 lines, -17 lines)			Patch
A	[revision details]	View	1 2	1 chunk	+2 lines, -0 lines	0 comments	Download
M	environs/instances/image.go	View		2 chunks	+25 lines, -8 lines	0 comments	Download
M	environs/instances/image_test.go	View	1 2	3 chunks	+33 lines, -1 line	0 comments	Download
M	provider/azure/environ.go	View	1	3 chunks	+19 lines, -2 lines	0 comments	Download
M	provider/azure/instancetype_test.go	View	1	2 chunks	+57 lines, -0 lines	0 comments	Download
M	provider/ec2/ec2.go	View	1 2	3 chunks	+33 lines, -1 line	0 comments	Download
M	provider/ec2/image_test.go	View		1 chunk	+12 lines, -0 lines	0 comments	Download
M	provider/ec2/local_test.go	View	1 2	1 chunk	+21 lines, -0 lines	0 comments	Download
M	provider/joyent/environ.go	View	1	4 chunks	+20 lines, -1 line	0 comments	Download
M	provider/joyent/environ_instance.go	View	1	3 chunks	+12 lines, -3 lines	0 comments	Download
M	provider/openstack/local_test.go	View	1 2	1 chunk	+33 lines, -0 lines	0 comments	Download
M	provider/openstack/provider.go	View		3 chunks	+20 lines, -1 line	0 comments	Download

Messages

Total messages: 5

Expand All Messages | Collapse All Messages

fwereade

Very nice, LGTM. https://codereview.appspot.com/87760055/diff/20001/environs/instances/image_test.go File environs/instances/image_test.go (right): https://codereview.appspot.com/87760055/diff/20001/environs/instances/image_test.go#newcode232 environs/instances/image_test.go:232: }, maybe a test with an ...

10 years ago (2014-04-18 09:28:34 UTC) #3

wallyworld

Please take a look. https://codereview.appspot.com/87760055/diff/20001/provider/ec2/ec2.go File provider/ec2/ec2.go (right): https://codereview.appspot.com/87760055/diff/20001/provider/ec2/ec2.go#newcode390 provider/ec2/ec2.go:390: } On 2014/04/18 09:28:35, fwereade ...

10 years ago (2014-04-18 13:59:57 UTC) #4

fwereade

10 years ago (2014-04-21 10:50:41 UTC) #5

https://codereview.appspot.com/87760055/diff/20001/provider/ec2/ec2.go
File provider/ec2/ec2.go (right):

https://codereview.appspot.com/87760055/diff/20001/provider/ec2/ec2.go#newcod...
provider/ec2/ec2.go:390: }
On 2014/04/18 13:59:57, wallyworld wrote:
> On 2014/04/18 09:28:35, fwereade wrote:
> > I'm wondering whether we should be checking a bit more here -- ie try to
> > actually get an image/instance-type pair, using the same codepaths we'll use
> > when actually running StartInstance. Crazy?
> 
> The image/instance pair stuff is done a bit later in the fetch image workflow.

Yeah, but that's the same delayed-error behaviour that's worrying me in the
other branch.

Don't get me wrong, all this is good incremental progress that ought to land,
but we need to keep track of what the eventual UX will be.

Expand All Messages | Collapse All Messages