)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"434ed4af29fa884f56a4fad2039be1373effabcb","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"The \u0027record\u0027 option of the WebSocketProxy should be string"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If nova-novncproxy is started with the \u0027record\u0027 option, an exception is"},{"line_number":10,"context_line":"thrown. This is because WebSocketProxy expects a file path for the"},{"line_number":11,"context_line":"record argument instead of boolean value. This patch fixes this by"},{"line_number":12,"context_line":"changing \u0027record\u0027 to be string option. An empty string means no"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"fa0719c6_8a8110c6","line":9,"range":{"start_line":9,"start_character":3,"end_line":9,"end_character":55},"updated":"2016-03-23 10:15:45.000000000","message":"Any proxy service of us which uses websockify will behave this way.\n\n    $ grep -n -r \"baseproxy.proxy\" nova/cmd/\n    nova/cmd/serialproxy.py:37:    baseproxy.proxy(\n    nova/cmd/spicehtml5proxy.py:47:    baseproxy.proxy(\n    nova/cmd/novncproxy.py:37:    baseproxy.proxy(","commit_id":"761a400890ab3496d774307c92491d8c2f4f1f4b"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"434ed4af29fa884f56a4fad2039be1373effabcb","unresolved":false,"context_lines":[{"line_number":9,"context_line":"If nova-novncproxy is started with the \u0027record\u0027 option, an exception is"},{"line_number":10,"context_line":"thrown. This is because WebSocketProxy expects a file path for the"},{"line_number":11,"context_line":"record argument instead of boolean value. This patch fixes this by"},{"line_number":12,"context_line":"changing \u0027record\u0027 to be string option. An empty string means no"},{"line_number":13,"context_line":"recording."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"`nova-novncproxy --help\u0027 now produces the following help message for"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"fa0719c6_0a9ea062","line":12,"range":{"start_line":12,"start_character":41,"end_line":12,"end_character":62},"updated":"2016-03-23 10:15:45.000000000","message":"\u003e An empty string means no recording\n\nPut this into the help text of the option.","commit_id":"761a400890ab3496d774307c92491d8c2f4f1f4b"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"434ed4af29fa884f56a4fad2039be1373effabcb","unresolved":false,"context_lines":[{"line_number":9,"context_line":"If nova-novncproxy is started with the \u0027record\u0027 option, an exception is"},{"line_number":10,"context_line":"thrown. This is because WebSocketProxy expects a file path for the"},{"line_number":11,"context_line":"record argument instead of boolean value. This patch fixes this by"},{"line_number":12,"context_line":"changing \u0027record\u0027 to be string option. An empty string means no"},{"line_number":13,"context_line":"recording."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"`nova-novncproxy --help\u0027 now produces the following help message for"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"fa0719c6_a7952d88","line":12,"range":{"start_line":12,"start_character":0,"end_line":12,"end_character":38},"updated":"2016-03-23 10:15:45.000000000","message":"Changing config options types (and with that their way of validation) needs a reno file [1] to make the deployers aware of that.\n\n[1] http://docs.openstack.org/developer/nova/code-review.html#when-a-release-note-is-needed","commit_id":"761a400890ab3496d774307c92491d8c2f4f1f4b"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"434ed4af29fa884f56a4fad2039be1373effabcb","unresolved":false,"context_lines":[{"line_number":12,"context_line":"changing \u0027record\u0027 to be string option. An empty string means no"},{"line_number":13,"context_line":"recording."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"`nova-novncproxy --help\u0027 now produces the following help message for"},{"line_number":16,"context_line":"this argument:"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"  --record RECORD       Record sessions to RECORD.[session_number]"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Closes-Bug: #1559026"},{"line_number":21,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"fa0719c6_8a975039","line":18,"range":{"start_line":15,"start_character":0,"end_line":18,"end_character":66},"updated":"2016-03-23 10:15:45.000000000","message":"nit: not really necessary IMO","commit_id":"761a400890ab3496d774307c92491d8c2f4f1f4b"}],"nova/cmd/novnc.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a65c27ee55c1bc2c40b2b7dd1d99b44599e4e0a1","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"opts \u003d ["},{"line_number":19,"context_line":"    cfg.StrOpt(\u0027record\u0027,"},{"line_number":20,"context_line":"                help\u003d\u0027Record sessions to FILE.[session_number]\u0027),"},{"line_number":21,"context_line":"    cfg.BoolOpt(\u0027daemon\u0027,"},{"line_number":22,"context_line":"                default\u003dFalse,"},{"line_number":23,"context_line":"                help\u003d\u0027Become a daemon (background process)\u0027),"}],"source_content_type":"text/x-python","patch_set":2,"id":"1af94dfe_bb14da4c","line":20,"range":{"start_line":20,"start_character":16,"end_line":20,"end_character":62},"updated":"2016-03-18 15:35:08.000000000","message":"Can we update this a bit to be more clear on what this is used for?\n\nI believe, from looking at the websockify 0.6.1 code (our min version in global-requirements), that the \u0027record\u0027 value is the filename, and \u0027session_number\u0027 is the handler id:\n\nhttps://github.com/kanaka/websockify/blob/v0.6.1/websockify/websocket.py#L483\n\nSo let\u0027s be clear that \u0027record\u0027 is a filename for recording websocket session logs.","commit_id":"2d5b28b885853384535e7b062c7021d661c7a171"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0aba38ea277c81b57da205256484f78e9ed8fbd1","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"opts \u003d ["},{"line_number":19,"context_line":"    cfg.StrOpt(\u0027record\u0027,"},{"line_number":20,"context_line":"                help\u003d\u0027Record sessions to FILE.[session_number]\u0027),"},{"line_number":21,"context_line":"    cfg.BoolOpt(\u0027daemon\u0027,"},{"line_number":22,"context_line":"                default\u003dFalse,"},{"line_number":23,"context_line":"                help\u003d\u0027Become a daemon (background process)\u0027),"}],"source_content_type":"text/x-python","patch_set":2,"id":"1af94dfe_16a7a5aa","line":20,"range":{"start_line":20,"start_character":16,"end_line":20,"end_character":62},"in_reply_to":"1af94dfe_bb14da4c","updated":"2016-03-18 15:43:54.000000000","message":"OK, the help was literally ripped from the websocketproxy command line:\n\nhttps://github.com/kanaka/websockify/blob/v0.6.1/websockify/websocketproxy.py#L325\n\nThe difference there is the FILE metavar is pretty clear. Let\u0027s just point out here that the record value is the FILE name.\n\nLater in newton it might be useful to actually rename this option, like session_recording_filename or something.","commit_id":"2d5b28b885853384535e7b062c7021d661c7a171"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"274a6ce7ffc47deff623caf5f34c71f33a0ede9b","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"opts \u003d ["},{"line_number":19,"context_line":"    cfg.StrOpt(\u0027record\u0027,"},{"line_number":20,"context_line":"                help\u003d\u0027Record sessions to RECORD.[session_number]\u0027),"},{"line_number":21,"context_line":"    cfg.BoolOpt(\u0027daemon\u0027,"},{"line_number":22,"context_line":"                default\u003dFalse,"},{"line_number":23,"context_line":"                help\u003d\u0027Become a daemon (background process)\u0027),"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa0719c6_0cac0e33","line":20,"updated":"2016-03-23 01:37:46.000000000","message":"meh, this still assumes the person looking at this config option is aware of how this is used or the websockify command line help text. I think we should at least point out that RECORD is a filename and websockify uses this to record the session in a file named RECORD.[session_number].\n\nMaybe markus_z has other suggestions here.","commit_id":"761a400890ab3496d774307c92491d8c2f4f1f4b"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"434ed4af29fa884f56a4fad2039be1373effabcb","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"opts \u003d ["},{"line_number":19,"context_line":"    cfg.StrOpt(\u0027record\u0027,"},{"line_number":20,"context_line":"                help\u003d\u0027Record sessions to RECORD.[session_number]\u0027),"},{"line_number":21,"context_line":"    cfg.BoolOpt(\u0027daemon\u0027,"},{"line_number":22,"context_line":"                default\u003dFalse,"},{"line_number":23,"context_line":"                help\u003d\u0027Become a daemon (background process)\u0027),"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa0719c6_e5d39d02","line":20,"in_reply_to":"fa0719c6_0cac0e33","updated":"2016-03-23 10:15:45.000000000","message":"This change here should focus on making the config option type correct to prevent the observed issue. The phrasing\ndoesn\u0027t help. Maybe something like:\n\n    This is the filename (or fully qualified path to a file)\n    which will store the websocket frames received and sent\n    by a proxy service (like VNC, SPICE, Serial) running\n    on this host. If this is *not* set (default), no\n    recording will be done.\n\nAt least that\u0027s how I understand [1] and [2].\n\nFollow up patches in this area here should:\n* rename the option \"record\" to \"session_recording_filename\"\n* move these options to a new group called \"proxy\" (or similar)\n* move them to \"/nova/conf/\"\n* provide a proper help text as proposed in blueprint centralize-config-options-newton (see [3] for additional details)\n\nReferences:\n[1] https://github.com/kanaka/websockify/blob/v0.6.1/websockify/websocket.py#L305\n[2] https://github.com/kanaka/websockify/blob/v0.6.1/websockify/websocket.py#L375\n[3] https://etherpad.openstack.org/p/config-options","commit_id":"761a400890ab3496d774307c92491d8c2f4f1f4b"}],"nova/tests/unit/cmd/test_baseproxy.py":[{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"434ed4af29fa884f56a4fad2039be1373effabcb","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        mock_init.assert_called_once_with("},{"line_number":62,"context_line":"            listen_host\u003d\u00270.0.0.0\u0027, listen_port\u003d\u00276080\u0027, source_is_ipv6\u003dFalse,"},{"line_number":63,"context_line":"            verbose\u003dFalse, cert\u003d\u0027self.pem\u0027, key\u003dNone, ssl_only\u003dFalse,"},{"line_number":64,"context_line":"            daemon\u003dFalse, record\u003dNone, traffic\u003dFalse,"},{"line_number":65,"context_line":"            web\u003d\u0027/usr/share/spice-html5\u0027, file_only\u003dTrue,"},{"line_number":66,"context_line":"            RequestHandlerClass\u003dwebsocketproxy.NovaProxyRequestHandler)"},{"line_number":67,"context_line":"        mock_start.assert_called_once_with()"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa0719c6_8742a966","line":64,"range":{"start_line":64,"start_character":25,"end_line":64,"end_character":38},"updated":"2016-03-23 10:15:45.000000000","message":"Just as a hint for other reviewers, websockify (v0.6.1) doesn\u0027t differ between \"\" and None: https://github.com/kanaka/websockify/blob/v0.6.1/websockify/websocket.py#L603","commit_id":"761a400890ab3496d774307c92491d8c2f4f1f4b"}],"releasenotes/notes/bug-1559026-47c3fa3468d66b07.yaml":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"81d20982e7e81ab409d465c796dff6f75da08204","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"upgrade:"},{"line_number":3,"context_line":"  - The ``record`` command line option for the proxy"},{"line_number":4,"context_line":"    services (like VNC, serial, spice) is changed from"},{"line_number":5,"context_line":"    boolean to string. It specifies the filename that will"},{"line_number":6,"context_line":"    be used for recording websocket frames."}],"source_content_type":"text/x-yaml","patch_set":5,"id":"ba0121b8_2c3416dc","line":3,"range":{"start_line":3,"start_character":47,"end_line":3,"end_character":52},"updated":"2016-04-01 02:14:34.000000000","message":"Maybe \u0027console proxy\u0027 to be more specific?","commit_id":"750bcf7a009a31c12a4c38a1e62c5fa1010d6660"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"8854006ca5c965be88e1779c75d44ff6228bf080","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"upgrade:"},{"line_number":3,"context_line":"  - The ``record`` command line option for the proxy"},{"line_number":4,"context_line":"    services (like VNC, serial, spice) is changed from"},{"line_number":5,"context_line":"    boolean to string. It specifies the filename that will"},{"line_number":6,"context_line":"    be used for recording websocket frames."}],"source_content_type":"text/x-yaml","patch_set":5,"id":"ba0121b8_5a829153","line":3,"range":{"start_line":3,"start_character":19,"end_line":3,"end_character":38},"updated":"2016-03-29 08:34:07.000000000","message":"nit: It\u0027s a config option (set be nova.conf) which can also be set as a command line option.","commit_id":"750bcf7a009a31c12a4c38a1e62c5fa1010d6660"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"81d20982e7e81ab409d465c796dff6f75da08204","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"upgrade:"},{"line_number":3,"context_line":"  - The ``record`` command line option for the proxy"},{"line_number":4,"context_line":"    services (like VNC, serial, spice) is changed from"},{"line_number":5,"context_line":"    boolean to string. It specifies the filename that will"},{"line_number":6,"context_line":"    be used for recording websocket frames."}],"source_content_type":"text/x-yaml","patch_set":5,"id":"ba0121b8_6c50ce0b","line":3,"range":{"start_line":3,"start_character":19,"end_line":3,"end_character":38},"in_reply_to":"ba0121b8_5a829153","updated":"2016-04-01 02:14:34.000000000","message":"Yeah let\u0027s say config option, command line option is confusing.","commit_id":"750bcf7a009a31c12a4c38a1e62c5fa1010d6660"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"085ba5bcd370a7f564ee6410a6e706663e4a2baa","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"upgrade:"},{"line_number":3,"context_line":"  - The ``record`` configuration option for the console proxy"},{"line_number":4,"context_line":"    services (like VNC, serial, spice) is changed from"},{"line_number":5,"context_line":"    boolean to string. It specifies the filename that will"}],"source_content_type":"text/x-yaml","patch_set":6,"id":"ba0121b8_44f788d2","line":2,"range":{"start_line":2,"start_character":0,"end_line":2,"end_character":7},"updated":"2016-04-01 14:20:39.000000000","message":"nit: I\u0027m not entirely sure this should be \u0027upgrade\u0027, it could be \u0027fixes\u0027 or \u0027other\u0027. It\u0027s not really an upgrade problem because if you were setting this to something besides a path before it wouldn\u0027t work, but you couldn\u0027t set it to be a string either.","commit_id":"c5311439d6526006dd1354e09f2bfb86505d550d"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"b4014a73e724a708ddaf6c06df803a1de83fc8af","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"upgrade:"},{"line_number":3,"context_line":"  - The ``record`` configuration option for the console proxy"},{"line_number":4,"context_line":"    services (like VNC, serial, spice) is changed from"},{"line_number":5,"context_line":"    boolean to string. It specifies the filename that will"}],"source_content_type":"text/x-yaml","patch_set":6,"id":"ba0121b8_6d142fe8","line":2,"range":{"start_line":2,"start_character":0,"end_line":2,"end_character":7},"in_reply_to":"ba0121b8_44f788d2","updated":"2016-04-01 15:10:07.000000000","message":"@Matt: There could be the (very rare) case that someone sets this config option to \"0\" or \"1\" (string). These would be considered as False or True from oslo.config [1]. So using \"upgrade\" here should be OK imo.\n\n[1] https://github.com/openstack/oslo.config/blob/3ea54921976388fb787fc576681da950319f57e8/oslo_config/types.py#L216-L217","commit_id":"c5311439d6526006dd1354e09f2bfb86505d550d"}]}