juju-core

Merge lp:~wallyworld/juju-core/manual-provider-storage-port-type into lp:~go-bot/juju-core/trunk

  1. manual-provider-storage-port-type
  2. Merge into trunk
Proposed by Ian Booth
Status: Merged
Approved by: Ian Booth
Approved revision: no longer in the source branch.
Merged at revision: 2588
Proposed branch: lp:~wallyworld/juju-core/manual-provider-storage-port-type
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 69 lines (+18/-5)
2 files modified
provider/manual/config.go (+2/-2)
provider/manual/config_test.go (+16/-3)
To merge this branch: bzr merge lp:~wallyworld/juju-core/manual-provider-storage-port-type
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+214866@code.launchpad.net

Commit message

Force manual storage-port to int

The manual provider storage-port config
attribute needs to have a schema definition
of ForceInt so it can be deserialised from
state correctly.

https://codereview.appspot.com/85750045/

Description of the change

Force manual storage-port to int

The manual provider storage-port config
attribute needs to have a schema definition
of ForceInt so it can be deserialised from
state correctly.

https://codereview.appspot.com/85750045/

To post a comment you must log in.
Revision history for this message
Ian Booth (wallyworld) wrote :
Download full text (3.6 KiB)

Reviewers: mp+214866_code.launchpad.net,

Message:
Please take a look.

Description:
Force manual storage-port to int

The manual provider storage-port config
attribute needs to have a schema definition
of ForceInt so it can be deserialised from
state correctly.

https://code.launchpad.net/~wallyworld/juju-core/manual-provider-storage-port-type/+merge/214866

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/85750045/

Affected files (+20, -5 lines):
   A [revision details]
   M provider/manual/config.go
   M provider/manual/config_test.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20140408170048-h58307lijtftb3su
+New revision: <email address hidden>

Index: provider/manual/config.go
=== modified file 'provider/manual/config.go'
--- provider/manual/config.go 2014-02-10 11:35:13 +0000
+++ provider/manual/config.go 2014-04-09 01:12:03 +0000
@@ -17,7 +17,7 @@
    "bootstrap-host": schema.String(),
    "bootstrap-user": schema.String(),
    "storage-listen-ip": schema.String(),
- "storage-port": schema.Int(),
+ "storage-port": schema.ForceInt(),
    "storage-auth-key": schema.String(),
    "use-sshstorage": schema.Bool(),
   }
@@ -59,7 +59,7 @@
  }

  func (c *environConfig) storagePort() int {
- return int(c.attrs["storage-port"].(int64))
+ return c.attrs["storage-port"].(int)
  }

  func (c *environConfig) storageAuthKey() string {

Index: provider/manual/config_test.go
=== modified file 'provider/manual/config_test.go'
--- provider/manual/config_test.go 2014-03-13 07:54:56 +0000
+++ provider/manual/config_test.go 2014-04-09 01:12:03 +0000
@@ -72,7 +72,7 @@
   c.Assert(unknownAttrs["bootstrap-host"], gc.Equals, "hostname")
   c.Assert(unknownAttrs["bootstrap-user"], gc.Equals, "")
   c.Assert(unknownAttrs["storage-listen-ip"], gc.Equals, "")
- c.Assert(unknownAttrs["storage-port"], gc.Equals, int64(8040))
+ c.Assert(unknownAttrs["storage-port"], gc.Equals, int(8040))
  }

  func (s *configSuite) TestConfigMutability(c *gc.C) {
@@ -89,7 +89,7 @@
    "bootstrap-host": "new-hostname",
    "bootstrap-user": "new-username",
    "storage-listen-ip": "10.0.0.123",
- "storage-port": int64(1234),
+ "storage-port": 1234,
   } {
    testConfig = MinimalConfig(c)
    testConfig, err = testConfig.Apply(map[string]interface{}{k: v})
@@ -119,7 +119,7 @@
   c.Assert(testConfig.storageAddr(), gc.Equals, "hostname:8040")
   c.Assert(testConfig.storageListenAddr(), gc.Equals, ":8040")
   values["storage-listen-ip"] = "10.0.0.123"
- values["storage-port"] = int64(1234)
+ values["storage-port"] = 1234
   testConfig = getEnvironConfig(c, values)
   c.Assert(testConfig.storageAddr(), gc.Equals, "hostname:1234")
   c.Assert(testConfig.storageListenAddr(), gc.Equals, "10.0.0.123:1234")
@@ -137,3 +137,16 @@
   c.Assert(err, gc.IsNil)
   c.Assert(envConfig.useSSHStorage(), jc.IsFalse)
  }
+
+func (s *configSuite) TestValidateConfigWithFloatPort(c *gc.C) {
+ // When the config values get seria...

Read more...

Revision history for this message
Andrew Wilkins (axwalk) wrote :

On 2014/04/09 01:22:19, wallyworld wrote:
> Please take a look.

LGTM, thanks. How has this been working?

https://codereview.appspot.com/85750045/

Revision history for this message
Ian Booth (wallyworld) wrote :

On 2014/04/09 01:23:34, axw wrote:
> On 2014/04/09 01:22:19, wallyworld wrote:
> > Please take a look.

> LGTM, thanks. How has this been working?

No idea :-)

https://codereview.appspot.com/85750045/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'provider/manual/config.go'
2--- provider/manual/config.go 2014-02-10 11:35:13 +0000
3+++ provider/manual/config.go 2014-04-09 01:14:42 +0000
4@@ -17,7 +17,7 @@
5 "bootstrap-host": schema.String(),
6 "bootstrap-user": schema.String(),
7 "storage-listen-ip": schema.String(),
8- "storage-port": schema.Int(),
9+ "storage-port": schema.ForceInt(),
10 "storage-auth-key": schema.String(),
11 "use-sshstorage": schema.Bool(),
12 }
13@@ -59,7 +59,7 @@
14 }
15
16 func (c *environConfig) storagePort() int {
17- return int(c.attrs["storage-port"].(int64))
18+ return c.attrs["storage-port"].(int)
19 }
20
21 func (c *environConfig) storageAuthKey() string {
22
23=== modified file 'provider/manual/config_test.go'
24--- provider/manual/config_test.go 2014-03-13 07:54:56 +0000
25+++ provider/manual/config_test.go 2014-04-09 01:14:42 +0000
26@@ -72,7 +72,7 @@
27 c.Assert(unknownAttrs["bootstrap-host"], gc.Equals, "hostname")
28 c.Assert(unknownAttrs["bootstrap-user"], gc.Equals, "")
29 c.Assert(unknownAttrs["storage-listen-ip"], gc.Equals, "")
30- c.Assert(unknownAttrs["storage-port"], gc.Equals, int64(8040))
31+ c.Assert(unknownAttrs["storage-port"], gc.Equals, int(8040))
32 }
33
34 func (s *configSuite) TestConfigMutability(c *gc.C) {
35@@ -89,7 +89,7 @@
36 "bootstrap-host": "new-hostname",
37 "bootstrap-user": "new-username",
38 "storage-listen-ip": "10.0.0.123",
39- "storage-port": int64(1234),
40+ "storage-port": 1234,
41 } {
42 testConfig = MinimalConfig(c)
43 testConfig, err = testConfig.Apply(map[string]interface{}{k: v})
44@@ -119,7 +119,7 @@
45 c.Assert(testConfig.storageAddr(), gc.Equals, "hostname:8040")
46 c.Assert(testConfig.storageListenAddr(), gc.Equals, ":8040")
47 values["storage-listen-ip"] = "10.0.0.123"
48- values["storage-port"] = int64(1234)
49+ values["storage-port"] = 1234
50 testConfig = getEnvironConfig(c, values)
51 c.Assert(testConfig.storageAddr(), gc.Equals, "hostname:1234")
52 c.Assert(testConfig.storageListenAddr(), gc.Equals, "10.0.0.123:1234")
53@@ -137,3 +137,16 @@
54 c.Assert(err, gc.IsNil)
55 c.Assert(envConfig.useSSHStorage(), jc.IsFalse)
56 }
57+
58+func (s *configSuite) TestValidateConfigWithFloatPort(c *gc.C) {
59+ // When the config values get serialized through JSON, the integers
60+ // get coerced to float64 values. The parsing needs to handle this.
61+ values := MinimalConfigValues()
62+ values["storage-port"] = float64(8040)
63+ cfg, err := config.New(config.UseDefaults, values)
64+ c.Assert(err, gc.IsNil)
65+ valid, err := ProviderInstance.Validate(cfg, nil)
66+ c.Assert(err, gc.IsNil)
67+ unknownAttrs := valid.UnknownAttrs()
68+ c.Assert(unknownAttrs["storage-port"], gc.Equals, int(8040))
69+}

Subscribers

People subscribed via source and target branches

to status/vote changes: