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

Issue 9824043: Move assignment policy to global env config (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years, 6 months ago by wallyworld
Modified:
6 years, 6 months ago
Reviewers:
mp+165983, jameinel, fwereade
Visibility:
Public.

Description

Move assignment policy to global env config Previoysly, each provider defined it's own AssignmentPolicy, which was hard coded to AssignNew. Now, it's been made an env setting with default AssignNew. With containers, it will be possible to add a unit to an existing machine (if not dirty) so this work is a step in that direction. https://code.launchpad.net/~wallyworld/juju-core/move-assignment-policy/+merge/165983 Requires: https://code.launchpad.net/~wallyworld/juju-core/add-machine-dirty-flag/+merge/165954 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 14

Patch Set 2 : Move assignment policy to global env config #

Patch Set 3 : Move assignment policy to global env config #

Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -50 lines) Patch
A [revision details] View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M environs/dummy/environs.go View 1 chunk +0 lines, -7 lines 0 comments Download
M environs/ec2/ec2.go View 1 2 1 chunk +0 lines, -11 lines 0 comments Download
M environs/ec2/local_test.go View 1 chunk +0 lines, -3 lines 0 comments Download
M environs/interface.go View 1 2 1 chunk +0 lines, -3 lines 0 comments Download
M environs/maas/environ.go View 1 2 1 chunk +0 lines, -4 lines 0 comments Download
M environs/maas/environ_test.go View 1 2 1 chunk +0 lines, -6 lines 0 comments Download
M environs/openstack/local_test.go View 1 chunk +0 lines, -3 lines 0 comments Download
M environs/openstack/provider.go View 1 2 1 chunk +0 lines, -9 lines 0 comments Download
M juju/conn.go View 1 2 1 chunk +3 lines, -4 lines 0 comments Download

Messages

Total messages: 11
wallyworld
Please take a look.
6 years, 6 months ago (2013-05-28 08:09:41 UTC) #1
jameinel
I agree with the principle that the assignment policy is not based on the provider ...
6 years, 6 months ago (2013-05-28 11:37:20 UTC) #2
fwereade
NOT LGTM, we should have a talk about how we implement this. I presume there's ...
6 years, 6 months ago (2013-05-28 12:16:48 UTC) #3
wallyworld
Sorry for not being clear that the end goal is to use constraints for the ...
6 years, 6 months ago (2013-05-29 03:57:11 UTC) #4
fwereade
A few more comments. https://codereview.appspot.com/9824043/diff/1/environs/config/config.go File environs/config/config.go (right): https://codereview.appspot.com/9824043/diff/1/environs/config/config.go#newcode47 environs/config/config.go:47: AssignUnused AssignmentPolicy = "unused" On ...
6 years, 6 months ago (2013-05-30 15:32:58 UTC) #5
wallyworld
I've removed all the code that put assignment policy in the environment config. All this ...
6 years, 6 months ago (2013-05-31 04:42:46 UTC) #6
wallyworld
Please take a look.
6 years, 6 months ago (2013-05-31 04:44:26 UTC) #7
fwereade
On 2013/05/31 04:44:26, wallyworld wrote: > Please take a look. LGTM trivial in its current ...
6 years, 6 months ago (2013-05-31 09:02:43 UTC) #8
jameinel
LGTM
6 years, 6 months ago (2013-05-31 13:10:21 UTC) #9
jameinel
On 2013/05/31 13:10:21, jameinel wrote: > LGTM Just make sure to update the cover description ...
6 years, 6 months ago (2013-05-31 13:11:12 UTC) #10
wallyworld
6 years, 6 months ago (2013-06-10 10:42:49 UTC) #11
*** Submitted:

Move assignment policy to global env config

Previoysly, each provider defined it's own AssignmentPolicy,
which was hard coded to AssignNew. Now, it's been made an
env setting with default AssignNew. With containers, it will be
possible to add a unit to an existing machine (if not dirty) so
this work is a step in that direction.

R=jameinel, fwereade
CC=
https://codereview.appspot.com/9824043
Sign in to reply to this message.

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