Update patch set 5

Patch Set 5:

(3 comments)

Patch-set: 5
Label: Verified=0

c200c8997fcb05b817902c6c70a69c0b11152a04

@@ -0,0 +1,73 @@
+{
+  "comments": [
+    {
+      "key": {
+        "uuid": "5ff73747_c42a6bd9",
+        "filename": "specs/newton/address-scope-subnet-pool-mapping.rst",
+        "patchSetId": 5
+      },
+      "lineNbr": 153,
+      "author": {
+        "id": 1689
+      },
+      "writtenOn": "2017-04-28T17:35:56Z",
+      "side": 1,
+      "message": "I think this text should be replaced with something like \"... made up of the prefixes attribute values of all subnetpools associated with the l3_policy.\"\n\nI\u0027m pretty sure we don\u0027t want to include prefixes from subnetpools associated with the address scope if they are not also associated with this l3_policy.",
+      "range": {
+        "startLine": 152,
+        "startChar": 58,
+        "endLine": 153,
+        "endChar": 49
+      },
+      "revId": "c200c8997fcb05b817902c6c70a69c0b11152a04",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    },
+    {
+      "key": {
+        "uuid": "5ff73747_c7d38d03",
+        "filename": "specs/newton/address-scope-subnet-pool-mapping.rst",
+        "patchSetId": 5
+      },
+      "lineNbr": 273,
+      "author": {
+        "id": 1689
+      },
+      "writtenOn": "2017-04-28T17:35:56Z",
+      "side": 1,
+      "message": "We could write a validator that accepts either a single CIDR or a list of CIDRs. Validating in the plugin is OK for now, but we should be certain that the same JSON would be accepted either way. One question is whether JSON list syntax would get through this string validation, and if not, if there is some way to disable the validation here completely. I\u0027m not very familiar with how the JSON parsing and validation works, so I\u0027m hoping others are reviewing this closely. \n\nWe should make sure that lists will be accepted and that backwards compatibility will be retained for arbitrary REST clients, for the GBP python client and CLI, and for horizon (if that supports creating L3Ps).\n\nOne other concern is that the DB layer currently persists ip_pool as sa.String(64), which may not be big enough for a list. Maybe the _make_l3_policy_dict method should not use this DB column, and instead build the the ip_pool value for the result from the prefixes attribute values of all the associated subnetpools. I think this is the only correct way to do this, since the prefixes attribute of subnetpool is mutable.\n\nIf all the above gets too complicated or problematic, maybe we should consider instead adding a new ip_pools attribute to replace ip_pool that uses type:subnet_list for validation and is always a list. We could then change the ip_pool validator to type:subnet_or_none, and allow passing in either ip_pool or ip_pools, but not both.\n\nI\u0027m not sure which approach I prefer.",
+      "range": {
+        "startLine": 273,
+        "startChar": 34,
+        "endLine": 273,
+        "endChar": 45
+      },
+      "revId": "c200c8997fcb05b817902c6c70a69c0b11152a04",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    },
+    {
+      "key": {
+        "uuid": "5ff73747_04e9737e",
+        "filename": "specs/newton/address-scope-subnet-pool-mapping.rst",
+        "patchSetId": 5
+      },
+      "lineNbr": 278,
+      "author": {
+        "id": 1689
+      },
+      "writtenOn": "2017-04-28T17:35:56Z",
+      "side": 1,
+      "message": "It seems we should remove this and possibly note that the attribute does not apply to IPv6 subnets.",
+      "range": {
+        "startLine": 278,
+        "startChar": 33,
+        "endLine": 278,
+        "endChar": 69
+      },
+      "revId": "c200c8997fcb05b817902c6c70a69c0b11152a04",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    }
+  ]
+}