Update patch set 3

Patch Set 3: Code-Review-1

(7 comments)

Patch-set: 3
Reviewer: Gerrit User 13252 <13252@4a232e18-c5a9-48ee-94c0-e04e7cca6543>
Label: Code-Review=-1
This commit is contained in:
Gerrit User 13252 2018-07-12 14:14:16 +00:00 committed by Gerrit Code Review
parent 46c3b10afb
commit a855056aa9
2 changed files with 133 additions and 0 deletions

View File

@ -0,0 +1,21 @@
{
"comments": [
{
"key": {
"uuid": "5f7c97a3_4a485561",
"filename": "recipes/common.rb",
"patchSetId": 3
},
"lineNbr": 82,
"author": {
"id": 13252
},
"writtenOn": "2018-07-12T14:14:16Z",
"side": 0,
"message": "This config option is deprecated, but instead of dropping it, we should set \u0027www_authenticate_uri\u0027 instead. And for this option the public endpoint is indeed the one that is correct, because it will be presented to the non-authenticated user.",
"revId": "59112747e865faeeb84b92e189bfd84b7fb0bc04",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
}
]
}

View File

@ -0,0 +1,112 @@
{
"comments": [
{
"key": {
"uuid": "5f7c97a3_6aa191a8",
"filename": "attributes/default.rb",
"patchSetId": 3
},
"lineNbr": 27,
"author": {
"id": 13252
},
"writtenOn": "2018-07-12T14:14:16Z",
"side": 1,
"message": "I think this is wrong, the plain endpoint should not need to contain the version information.",
"revId": "d9fffcbd1740cc207b29edde29fceba3299fe493",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "5f7c97a3_2a22b9a4",
"filename": "recipes/common.rb",
"patchSetId": 3
},
"lineNbr": 73,
"author": {
"id": 13252
},
"writtenOn": "2018-07-12T14:14:16Z",
"side": 1,
"message": "Actually it is a bit debatable whether internal services (from a user pov) should be using the public or the internal endpoint. Instead of hardcoding the public endpoint, which to me seems like a step into the wrong direction, we should stick to the internal one in my opinionated view.\n\nAn alternative would be to make this choice configurable for the deployer, but I\u0027m not sure that that added complexity is useful.",
"revId": "d9fffcbd1740cc207b29edde29fceba3299fe493",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "5f7c97a3_0a3c9d90",
"filename": "recipes/identity_registration.rb",
"patchSetId": 3
},
"lineNbr": 27,
"author": {
"id": 13252
},
"writtenOn": "2018-07-12T14:14:16Z",
"side": 1,
"message": "Same thing here, I\u0027d rather use the internal endpoint.",
"revId": "d9fffcbd1740cc207b29edde29fceba3299fe493",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "5f7c97a3_6a0511c1",
"filename": "recipes/identity_registration.rb",
"patchSetId": 3
},
"lineNbr": 28,
"author": {
"id": 13252
},
"writtenOn": "2018-07-12T14:14:16Z",
"side": 1,
"message": "I think we should drop these transformations everywhere, just use the version we have in the endpoint. But that may be better done in a separate patch series that fixes this consistently for all cookbooks.",
"revId": "d9fffcbd1740cc207b29edde29fceba3299fe493",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "5f7c97a3_cae1c5ff",
"filename": "recipes/neutron_int.rb",
"patchSetId": 3
},
"lineNbr": 26,
"author": {
"id": 13252
},
"writtenOn": "2018-07-12T14:14:16Z",
"side": 1,
"message": "I\u0027m thinking that this may add the version path twice. It is really only needed because neutron lacks the ability to do version discovery properly. I\u0027d rather leave this the original version.",
"revId": "d9fffcbd1740cc207b29edde29fceba3299fe493",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "5f7c97a3_caca256e",
"filename": "recipes/neutron_int.rb",
"patchSetId": 3
},
"lineNbr": 29,
"author": {
"id": 13252
},
"writtenOn": "2018-07-12T14:14:16Z",
"side": 1,
"message": "Same comments as for other recipes apply.",
"range": {
"startLine": 28,
"startChar": 0,
"endLine": 29,
"endChar": 97
},
"revId": "d9fffcbd1740cc207b29edde29fceba3299fe493",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
}
]
}