Update patch set 1

Patch Set 1: Code-Review-1 (9 comments) Heather, thank you for your patch. I've made a couple of comments (some with examples) after looking at it - please take a look. Patch-set: 1 Reviewer: Gerrit User 24824 <24824@4a232e18-c5a9-48ee-94c0-e04e7cca6543> Label: Code-Review=-1
2017-03-07 15:11:51 +00:00 · 2017-03-07 15:11:51 +00:00 · 3cf5e5fba7
parent 62c5632c8a
commit 3cf5e5fba7
1 changed files with 211 additions and 0 deletions
--- a/211
+++ b/211
@ -0,0 +1,211 @@
+{
+  "comments": [
+    {
+      "key": {
+        "uuid": "ba2be162_28e975b7",
+        "filename": "src/lib/charm/openstack/trove.py",
+        "patchSetId": 1
+      },
+      "lineNbr": 233,
+      "author": {
+        "id": 24824
+      },
+      "writtenOn": "2017-03-07T15:11:51Z",
+      "side": 1,
+      "message": "Please make sure to use network spaces functionality if possible in such cases, the code for that is present in core charms, e.g.:\n\nhttps://git.openstack.org/cgit/openstack/charm-keystone/tree/hooks/keystone_hooks.py#n297\n\nhttps://git.openstack.org/cgit/openstack/charm-keystone/commit/hooks/keystone_hooks.py?id\u003d063238f72d4948e2531df0cb5a42a85d361550e6\n\nThe respective juju hook tool is called network-get:\nhttps://github.com/juju/juju/blob/master/worker/uniter/runner/jujuc/network-get.go\n\nAnd the charm-helpers function for that is:\nhttps://pythonhosted.org/charmhelpers/api/charmhelpers.core.hookenv.html#charmhelpers.core.hookenv.network_get_primary_address",
+      "range": {
+        "startLine": 233,
+        "startChar": 0,
+        "endLine": 233,
+        "endChar": 53
+      },
+      "revId": "f39508b63f40edbb12c09bdf0740564814efae9a",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    },
+    {
+      "key": {
+        "uuid": "ba2be162_185cfc7b",
+        "filename": "src/templates/mitaka/trove-conductor.conf",
+        "patchSetId": 1
+      },
+      "lineNbr": 11,
+      "author": {
+        "id": 24824
+      },
+      "writtenOn": "2017-03-07T15:11:51Z",
+      "side": 1,
+      "message": "http://host:port/v2.0\nbut http://host:port/v3 - not 3.0.\n\nSee:\nhttps://docs.openstack.org/developer/python-keystoneclient/using-api-v3.html\n----\nhttps://developer.openstack.org/api-ref/identity/v2/index.html#\nhttps://developer.openstack.org/api-ref/identity/v3/index.html",
+      "range": {
+        "startLine": 11,
+        "startChar": 125,
+        "endLine": 11,
+        "endChar": 162
+      },
+      "revId": "f39508b63f40edbb12c09bdf0740564814efae9a",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    },
+    {
+      "key": {
+        "uuid": "9a30ddce_270c1b94",
+        "filename": "src/templates/mitaka/trove-guestagent.conf",
+        "patchSetId": 1
+      },
+      "lineNbr": 16,
+      "author": {
+        "id": 24824
+      },
+      "writtenOn": "2017-03-07T15:11:51Z",
+      "side": 1,
+      "message": "Shouldn\u0027t this be configurable for other datastore_manager values?\n\nhttps://github.com/openstack/trove/blob/57148d701d80823add450c9c44d9e9e188cfb946/trove/common/cfg.py#L1592\n\nI can see many of other alternatives so it probably should be a config option with a default option of \u0027mysql\u0027.\n\nhttps://github.com/openstack/trove/tree/e374c4966f58dce04e6de0732e21bc295a795a61/trove/guestagent/datastore/experimental",
+      "range": {
+        "startLine": 16,
+        "startChar": 18,
+        "endLine": 16,
+        "endChar": 19
+      },
+      "revId": "f39508b63f40edbb12c09bdf0740564814efae9a",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    },
+    {
+      "key": {
+        "uuid": "9a30ddce_92aa5ed3",
+        "filename": "src/templates/mitaka/trove-taskmanager.conf",
+        "patchSetId": 1
+      },
+      "lineNbr": 15,
+      "author": {
+        "id": 24824
+      },
+      "writtenOn": "2017-03-07T15:11:51Z",
+      "side": 1,
+      "message": "Are you sure it is a string \"None\" and not NoneType?",
+      "range": {
+        "startLine": 15,
+        "startChar": 42,
+        "endLine": 15,
+        "endChar": 48
+      },
+      "revId": "f39508b63f40edbb12c09bdf0740564814efae9a",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    },
+    {
+      "key": {
+        "uuid": "9a30ddce_17f02090",
+        "filename": "src/templates/mitaka/trove-taskmanager.conf",
+        "patchSetId": 1
+      },
+      "lineNbr": 24,
+      "author": {
+        "id": 24824
+      },
+      "writtenOn": "2017-03-07T15:11:51Z",
+      "side": 1,
+      "message": "See the previous comment on v2.0 vs v3.",
+      "range": {
+        "startLine": 24,
+        "startChar": 122,
+        "endLine": 24,
+        "endChar": 162
+      },
+      "revId": "f39508b63f40edbb12c09bdf0740564814efae9a",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    },
+    {
+      "key": {
+        "uuid": "9a30ddce_f2347adf",
+        "filename": "src/templates/mitaka/trove.conf",
+        "patchSetId": 1
+      },
+      "lineNbr": 14,
+      "author": {
+        "id": 24824
+      },
+      "writtenOn": "2017-03-07T15:11:51Z",
+      "side": 1,
+      "message": "Are you sure it is a string \"None\", and not NoneType?",
+      "range": {
+        "startLine": 14,
+        "startChar": 14,
+        "endLine": 14,
+        "endChar": 38
+      },
+      "revId": "f39508b63f40edbb12c09bdf0740564814efae9a",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    },
+    {
+      "key": {
+        "uuid": "9a30ddce_92f89ee7",
+        "filename": "src/templates/newton/trove-conductor.conf",
+        "patchSetId": 1
+      },
+      "lineNbr": 10,
+      "author": {
+        "id": 24824
+      },
+      "writtenOn": "2017-03-07T15:11:51Z",
+      "side": 1,
+      "message": "v2.0 vs v3 as in the previous comments.",
+      "range": {
+        "startLine": 10,
+        "startChar": 91,
+        "endLine": 10,
+        "endChar": 162
+      },
+      "revId": "f39508b63f40edbb12c09bdf0740564814efae9a",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    },
+    {
+      "key": {
+        "uuid": "9a30ddce_d2c13682",
+        "filename": "src/templates/newton/trove-guestagent.conf",
+        "patchSetId": 1
+      },
+      "lineNbr": 14,
+      "author": {
+        "id": 24824
+      },
+      "writtenOn": "2017-03-07T15:11:51Z",
+      "side": 1,
+      "message": "As in the previous comments.",
+      "range": {
+        "startLine": 14,
+        "startChar": 125,
+        "endLine": 14,
+        "endChar": 162
+      },
+      "revId": "f39508b63f40edbb12c09bdf0740564814efae9a",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    },
+    {
+      "key": {
+        "uuid": "9a30ddce_52bf06d0",
+        "filename": "src/templates/newton/trove.conf",
+        "patchSetId": 1
+      },
+      "lineNbr": 10,
+      "author": {
+        "id": 24824
+      },
+      "writtenOn": "2017-03-07T15:11:51Z",
+      "side": 1,
+      "message": "Same as before. Note that there is no .0 this time so 2.0 will be broken in this case.",
+      "range": {
+        "startLine": 10,
+        "startChar": 124,
+        "endLine": 10,
+        "endChar": 160
+      },
+      "revId": "f39508b63f40edbb12c09bdf0740564814efae9a",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    }
+  ]
+}