)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"a98928ac9b497962ff0e062458182d6a43680167","unresolved":false,"context_lines":[{"line_number":37,"context_line":"    \"\"\"Changes the owner and group of a given file."},{"line_number":38,"context_line":"    The call is a no-op if neither user nor group is provided.\"\"\""},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"Most occurrences of the above two shell calls (\u0027mkdir\u0027 and \u0027chown\u0027)"},{"line_number":41,"context_line":"in the guestagent code have been replaced by the above abstraction."},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"Change-Id: I0fe96301087e3bd49d937c7c489006fd9d84ca1e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7a75e904_350a1ad3","line":40,"updated":"2015-03-21 10:06:41.000000000","message":"I\u0027m curious why \u0027most\u0027 and not \u0027all\u0027. It would be good if you could provide some more information about this.","commit_id":"cc953ace90ee666a3c3a4e57b9934b9496bdb0bc"},{"author":{"_account_id":14576,"name":"Petr Malik","email":"pmalik@tesora.com","username":"pmalik"},"change_message_id":"6c6f92c16b713b19a4319c0b4b63cdb59d59105b","unresolved":false,"context_lines":[{"line_number":37,"context_line":"    \"\"\"Changes the owner and group of a given file."},{"line_number":38,"context_line":"    The call is a no-op if neither user nor group is provided.\"\"\""},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"Most occurrences of the above two shell calls (\u0027mkdir\u0027 and \u0027chown\u0027)"},{"line_number":41,"context_line":"in the guestagent code have been replaced by the above abstraction."},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"Change-Id: I0fe96301087e3bd49d937c7c489006fd9d84ca1e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"1a6ced46_7059a7fd","line":40,"in_reply_to":"7a75e904_350a1ad3","updated":"2015-03-24 16:47:10.000000000","message":"I did not replace native Python os.mkdir() / os.chown() calls - calls that do not require root. We may chose to do all this way though...\nI also did not replace calls that are part of larger \u0027scripts\u0027 executed line-by-line - see couchbase_impl for instance.","commit_id":"cc953ace90ee666a3c3a4e57b9934b9496bdb0bc"}],"trove/guestagent/common/operating_system.py":[{"author":{"_account_id":15321,"name":"Matthew Van Dijk","email":"mvandijk@tesora.com","username":"mvandijk"},"change_message_id":"11524e99163f2ce8954078427f6c451eb6942d61","unresolved":false,"context_lines":[{"line_number":111,"context_line":"    return addresses_by_family.pop()[\u0027addr\u0027]"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"def create_directory(dir_path, user\u003dNone, group\u003dNone, force\u003dTrue):"},{"line_number":115,"context_line":"    \"\"\"Create a given directory as root and update its ownership"},{"line_number":116,"context_line":"    (recursively) to the given user and group if any."},{"line_number":117,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9a80dd14_b25bdd52","line":114,"updated":"2015-03-13 14:35:08.000000000","message":"Would there be a case where we want to set the mode (-m) of the directory?","commit_id":"cc953ace90ee666a3c3a4e57b9934b9496bdb0bc"},{"author":{"_account_id":14576,"name":"Petr Malik","email":"pmalik@tesora.com","username":"pmalik"},"change_message_id":"d0229d36765d28161f9fa70d733ff3873c96013e","unresolved":false,"context_lines":[{"line_number":111,"context_line":"    return addresses_by_family.pop()[\u0027addr\u0027]"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"def create_directory(dir_path, user\u003dNone, group\u003dNone, force\u003dTrue):"},{"line_number":115,"context_line":"    \"\"\"Create a given directory as root and update its ownership"},{"line_number":116,"context_line":"    (recursively) to the given user and group if any."},{"line_number":117,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9a80dd14_c87fbd15","line":114,"in_reply_to":"9a80dd14_b25bdd52","updated":"2015-03-13 15:46:02.000000000","message":"An interesting question. I have not encounter such a call in the guestagent.\nI meant this mainly as the first-pass cleanup of the guestagent code. I am not sure if we want to start implementing functionality that may not be required.\n\nAnyway, should not be that difficult to add it as an optional dictionary parameter any time later.\nMaybe something like:\nmodes \u003d {\u0027u\u0027: \u0027\u003drwx\u0027, \u0027g\u0027: \u0027\u003drx\u0027, \u0027o\u0027: \u0027+r\u0027}","commit_id":"cc953ace90ee666a3c3a4e57b9934b9496bdb0bc"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"a98928ac9b497962ff0e062458182d6a43680167","unresolved":false,"context_lines":[{"line_number":139,"context_line":"            _(\"Cannot create a blank directory.\"))"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"def update_owner(path, user, group, recursive\u003dTrue, force\u003dFalse):"},{"line_number":143,"context_line":"    \"\"\"Changes the owner and group of a given file."},{"line_number":144,"context_line":"    The call is a no-op if neither user nor group is provided."},{"line_number":145,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7a75e904_951f4e09","line":142,"updated":"2015-03-21 10:06:41.000000000","message":"I\u0027d suggest you add a mechanism to pass a timeout. Especially since recursive is allowed, it would be good to have a kwarg for timeout to make this routine more generic.\n\nJust because every caller you replaced had a timeout of default (or explicitly specified 30, the default) doesn\u0027t make this moot.","commit_id":"cc953ace90ee666a3c3a4e57b9934b9496bdb0bc"},{"author":{"_account_id":14576,"name":"Petr Malik","email":"pmalik@tesora.com","username":"pmalik"},"change_message_id":"6c6f92c16b713b19a4319c0b4b63cdb59d59105b","unresolved":false,"context_lines":[{"line_number":139,"context_line":"            _(\"Cannot create a blank directory.\"))"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"def update_owner(path, user, group, recursive\u003dTrue, force\u003dFalse):"},{"line_number":143,"context_line":"    \"\"\"Changes the owner and group of a given file."},{"line_number":144,"context_line":"    The call is a no-op if neither user nor group is provided."},{"line_number":145,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1a6ced46_d0aa5bd1","line":142,"in_reply_to":"7a75e904_951f4e09","updated":"2015-03-24 16:47:10.000000000","message":"I agree and will add an optional timeout parameter.","commit_id":"cc953ace90ee666a3c3a4e57b9934b9496bdb0bc"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"a98928ac9b497962ff0e062458182d6a43680167","unresolved":false,"context_lines":[{"line_number":169,"context_line":"                                       options, owner_group_modifier, path)"},{"line_number":170,"context_line":"    else:"},{"line_number":171,"context_line":"        raise exception.UnprocessableEntity("},{"line_number":172,"context_line":"            _(\"Cannot change ownership of a blank file.\"))"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"def _execute_shell_cmd_as_root(cmd, options, *args):"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a75e904_f5688289","line":172,"updated":"2015-03-21 10:06:41.000000000","message":"or directory","commit_id":"cc953ace90ee666a3c3a4e57b9934b9496bdb0bc"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"a98928ac9b497962ff0e062458182d6a43680167","unresolved":false,"context_lines":[{"line_number":179,"context_line":""},{"line_number":180,"context_line":"    cmd_flags \u003d _build_command_options(options)"},{"line_number":181,"context_line":"    cmd_args \u003d cmd_flags + list(args)"},{"line_number":182,"context_line":"    utils.execute_with_timeout(cmd, *cmd_args,"},{"line_number":183,"context_line":"                               run_as_root\u003dTrue, root_helper\u003d\u0027sudo\u0027)"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7a75e904_f5ffa2a8","line":182,"updated":"2015-03-21 10:06:41.000000000","message":"some callers used to be able to specify timeouts. Looks like this change kills that.","commit_id":"cc953ace90ee666a3c3a4e57b9934b9496bdb0bc"},{"author":{"_account_id":7806,"name":"Sushil Kumar","email":"skm.net@gmail.com","username":"sushilkm"},"change_message_id":"edc9c7e532c72ac459f2e736d2408e3ec8782f59","unresolved":false,"context_lines":[{"line_number":165,"context_line":"    if path:"},{"line_number":166,"context_line":"        if user or group:"},{"line_number":167,"context_line":"            owner_group_modifier \u003d _build_user_group_pair(user, group)"},{"line_number":168,"context_line":"            options \u003d ((\u0027f\u0027, force), (\u0027R\u0027, recursive))"},{"line_number":169,"context_line":"            _execute_shell_cmd(\u0027chown\u0027, options, owner_group_modifier, path,"},{"line_number":170,"context_line":"                               **kwargs)"},{"line_number":171,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"7aaa499b_e5346cdf","line":168,"updated":"2015-04-13 08:27:30.000000000","message":"There is no force option with chown \"os\" command,\ninstead, the \"-f\" option that a chown command has actually suppresses the errors, which would stop the method to raise exception in case chown fails.","commit_id":"662b89977bbe6e9637e21c1d1ecfcbb8975ed94b"},{"author":{"_account_id":7806,"name":"Sushil Kumar","email":"skm.net@gmail.com","username":"sushilkm"},"change_message_id":"9635a489eebc98793ddfa93ad55ed03b9176dca6","unresolved":false,"context_lines":[{"line_number":165,"context_line":"    if path:"},{"line_number":166,"context_line":"        if user or group:"},{"line_number":167,"context_line":"            owner_group_modifier \u003d _build_user_group_pair(user, group)"},{"line_number":168,"context_line":"            options \u003d ((\u0027f\u0027, force), (\u0027R\u0027, recursive))"},{"line_number":169,"context_line":"            _execute_shell_cmd(\u0027chown\u0027, options, owner_group_modifier, path,"},{"line_number":170,"context_line":"                               **kwargs)"},{"line_number":171,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"7aaa499b_76036643","line":168,"in_reply_to":"7aaa499b_167542f8","updated":"2015-04-13 16:20:03.000000000","message":"ubuntu@contributor:~/test$ chmod 777 -f testing\n\nubuntu@contributor:~/test$ ls -ltr\n\ntotal 0\n\n-rw-rw-r-- 1 root root 0 Apr 13 16:16 testing\n\nubuntu@contributor:~/test$ chmod 777  testing\n\nchmod: changing permissions of `testing\u0027: Operation not permitted","commit_id":"662b89977bbe6e9637e21c1d1ecfcbb8975ed94b"},{"author":{"_account_id":14576,"name":"Petr Malik","email":"pmalik@tesora.com","username":"pmalik"},"change_message_id":"a70e1e5a1c784662d7adc88a87186e24e396cec4","unresolved":false,"context_lines":[{"line_number":165,"context_line":"    if path:"},{"line_number":166,"context_line":"        if user or group:"},{"line_number":167,"context_line":"            owner_group_modifier \u003d _build_user_group_pair(user, group)"},{"line_number":168,"context_line":"            options \u003d ((\u0027f\u0027, force), (\u0027R\u0027, recursive))"},{"line_number":169,"context_line":"            _execute_shell_cmd(\u0027chown\u0027, options, owner_group_modifier, path,"},{"line_number":170,"context_line":"                               **kwargs)"},{"line_number":171,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"7aaa499b_213d8b2b","line":168,"in_reply_to":"7aaa499b_76036643","updated":"2015-04-13 21:45:54.000000000","message":"Yes, but here, we are not testing if the path exists or we have permissions to execute the command. We are testing if it\u0027s None.\nPassing in None for the path most likely is a coding error.","commit_id":"662b89977bbe6e9637e21c1d1ecfcbb8975ed94b"},{"author":{"_account_id":7806,"name":"Sushil Kumar","email":"skm.net@gmail.com","username":"sushilkm"},"change_message_id":"331e83a380b9b4a8b4bdae21f1eb2c215279b958","unresolved":false,"context_lines":[{"line_number":165,"context_line":"    if path:"},{"line_number":166,"context_line":"        if user or group:"},{"line_number":167,"context_line":"            owner_group_modifier \u003d _build_user_group_pair(user, group)"},{"line_number":168,"context_line":"            options \u003d ((\u0027f\u0027, force), (\u0027R\u0027, recursive))"},{"line_number":169,"context_line":"            _execute_shell_cmd(\u0027chown\u0027, options, owner_group_modifier, path,"},{"line_number":170,"context_line":"                               **kwargs)"},{"line_number":171,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"7aaa499b_167542f8","line":168,"in_reply_to":"7aaa499b_b317c3ec","updated":"2015-04-13 16:18:16.000000000","message":"ubuntu@contributor:~/test$ ls -ltr\n\ntotal 0\n\n-rw-rw-r-- 1 root root 0 Apr 13 16:16 testing\n\nubuntu@contributor:~/test$ chmod 777 test\n\nchmod: cannot access `test\u0027: No such file or directory\n\nubuntu@contributor:~/test$ chmod -f 777 test\n\nubuntu@contributor:~/test$ ls -ltr\n\ntotal 0\n\n-rw-rw-r-- 1 root root 0 Apr 13 16:16 testing","commit_id":"662b89977bbe6e9637e21c1d1ecfcbb8975ed94b"},{"author":{"_account_id":14576,"name":"Petr Malik","email":"pmalik@tesora.com","username":"pmalik"},"change_message_id":"b15b86daf7902fd2a0cedaaf82aedb93cccd4e7b","unresolved":false,"context_lines":[{"line_number":165,"context_line":"    if path:"},{"line_number":166,"context_line":"        if user or group:"},{"line_number":167,"context_line":"            owner_group_modifier \u003d _build_user_group_pair(user, group)"},{"line_number":168,"context_line":"            options \u003d ((\u0027f\u0027, force), (\u0027R\u0027, recursive))"},{"line_number":169,"context_line":"            _execute_shell_cmd(\u0027chown\u0027, options, owner_group_modifier, path,"},{"line_number":170,"context_line":"                               **kwargs)"},{"line_number":171,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"7aaa499b_b317c3ec","line":168,"in_reply_to":"7aaa499b_e5346cdf","updated":"2015-04-13 14:52:13.000000000","message":"Well, it says:\n-f, --silent, --quiet  suppress most error messages\n\nand it does not stop the exception in case of empty/missing target path.","commit_id":"662b89977bbe6e9637e21c1d1ecfcbb8975ed94b"},{"author":{"_account_id":10295,"name":"mariam john","email":"mariamj@us.ibm.com","username":"mariamj"},"change_message_id":"ceacc8778771aa1e824a0b9dc1da68b40859929f","unresolved":false,"context_lines":[{"line_number":220,"context_line":""},{"line_number":221,"context_line":"    :raises:                :class:`UnprocessableEntity` if dir_path not given."},{"line_number":222,"context_line":"    \"\"\""},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"    if dir_path:"},{"line_number":225,"context_line":"        _create_directory(dir_path, force, **kwargs)"},{"line_number":226,"context_line":"        chown(dir_path, user, group, **kwargs)"}],"source_content_type":"text/x-python","patch_set":12,"id":"7aaa499b_92a237ad","line":223,"updated":"2015-04-15 08:56:24.000000000","message":"Quick question Petr,  lets say user doesnt set run_as_root\u003dFalse and also doesnt set \u0027user\u0027. Shouldnt we check this here?","commit_id":"0d6e7181e97859c8c9ef1fb4834ccb543931b191"},{"author":{"_account_id":10295,"name":"mariam john","email":"mariamj@us.ibm.com","username":"mariamj"},"change_message_id":"827cd65f37da064d3be974176b416c57673eb59b","unresolved":false,"context_lines":[{"line_number":220,"context_line":""},{"line_number":221,"context_line":"    :raises:                :class:`UnprocessableEntity` if dir_path not given."},{"line_number":222,"context_line":"    \"\"\""},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"    if dir_path:"},{"line_number":225,"context_line":"        _create_directory(dir_path, force, **kwargs)"},{"line_number":226,"context_line":"        chown(dir_path, user, group, **kwargs)"}],"source_content_type":"text/x-python","patch_set":12,"id":"7aaa499b_486f5ec5","line":223,"in_reply_to":"7aaa499b_92a237ad","updated":"2015-04-15 09:58:53.000000000","message":"Please ignore this comment. I clearly need to get some sleep now.. Sorry about that. I can see that the user and group is processed in chown.","commit_id":"0d6e7181e97859c8c9ef1fb4834ccb543931b191"},{"author":{"_account_id":10295,"name":"mariam john","email":"mariamj@us.ibm.com","username":"mariamj"},"change_message_id":"ceacc8778771aa1e824a0b9dc1da68b40859929f","unresolved":false,"context_lines":[{"line_number":227,"context_line":"    else:"},{"line_number":228,"context_line":"        raise exception.UnprocessableEntity("},{"line_number":229,"context_line":"            _(\"Cannot create a blank directory.\"))"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"def chown(path, user, group, recursive\u003dTrue, force\u003dFalse, **kwargs):"},{"line_number":233,"context_line":"    \"\"\"Changes the owner and group of a given file."}],"source_content_type":"text/x-python","patch_set":12,"id":"7aaa499b_92b7d7d7","line":230,"updated":"2015-04-15 08:56:24.000000000","message":"I keep getting confused about whether to use _ to translate error messages we raise. There are some files where it doesnt translate the messages being raised and in some they do. Thanks Petr.","commit_id":"0d6e7181e97859c8c9ef1fb4834ccb543931b191"},{"author":{"_account_id":9782,"name":"Morgan Jones","email":"morgan@tesora.com","username":"vgnbkr"},"change_message_id":"abfcff3f12c5db2e84bcbea11be6cc7a498db8c9","unresolved":false,"context_lines":[{"line_number":227,"context_line":"    else:"},{"line_number":228,"context_line":"        raise exception.UnprocessableEntity("},{"line_number":229,"context_line":"            _(\"Cannot create a blank directory.\"))"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"def chown(path, user, group, recursive\u003dTrue, force\u003dFalse, **kwargs):"},{"line_number":233,"context_line":"    \"\"\"Changes the owner and group of a given file."}],"source_content_type":"text/x-python","patch_set":12,"id":"7aaa499b_abc36e56","line":230,"in_reply_to":"7aaa499b_92b7d7d7","updated":"2015-04-15 15:58:26.000000000","message":"FYI: My understanding is that debug messages should NOT be translated, all others should be.","commit_id":"0d6e7181e97859c8c9ef1fb4834ccb543931b191"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"bd7b9c0a08e8897947364604367fbe5a55039418","unresolved":false,"context_lines":[{"line_number":227,"context_line":"    else:"},{"line_number":228,"context_line":"        raise exception.UnprocessableEntity("},{"line_number":229,"context_line":"            _(\"Cannot create a blank directory.\"))"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"def chown(path, user, group, recursive\u003dTrue, force\u003dFalse, **kwargs):"},{"line_number":233,"context_line":"    \"\"\"Changes the owner and group of a given file."}],"source_content_type":"text/x-python","patch_set":12,"id":"7aaa499b_aeb93c17","line":230,"in_reply_to":"7aaa499b_abc36e56","updated":"2015-04-15 16:18:17.000000000","message":"Yep, that\u0027s the general policy","commit_id":"0d6e7181e97859c8c9ef1fb4834ccb543931b191"},{"author":{"_account_id":10295,"name":"mariam john","email":"mariamj@us.ibm.com","username":"mariamj"},"change_message_id":"205c78a35c21db41262839d5f60df7bc799031bd","unresolved":false,"context_lines":[{"line_number":227,"context_line":"    else:"},{"line_number":228,"context_line":"        raise exception.UnprocessableEntity("},{"line_number":229,"context_line":"            _(\"Cannot create a blank directory.\"))"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"def chown(path, user, group, recursive\u003dTrue, force\u003dFalse, **kwargs):"},{"line_number":233,"context_line":"    \"\"\"Changes the owner and group of a given file."}],"source_content_type":"text/x-python","patch_set":12,"id":"5aa745d1_cf529f13","line":230,"in_reply_to":"7aaa499b_aeb93c17","updated":"2015-04-16 18:59:46.000000000","message":"Thanks Peter \u0026 Morgan for clarifying.","commit_id":"0d6e7181e97859c8c9ef1fb4834ccb543931b191"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"bd7b9c0a08e8897947364604367fbe5a55039418","unresolved":false,"context_lines":[{"line_number":258,"context_line":"            owner_group_modifier \u003d _build_user_group_pair(user, group)"},{"line_number":259,"context_line":"            options \u003d ((\u0027f\u0027, force), (\u0027R\u0027, recursive))"},{"line_number":260,"context_line":"            _execute_shell_cmd(\u0027chown\u0027, options, owner_group_modifier, path,"},{"line_number":261,"context_line":"                               **kwargs)"},{"line_number":262,"context_line":"    else:"},{"line_number":263,"context_line":"        raise exception.UnprocessableEntity("},{"line_number":264,"context_line":"            _(\"Cannot change ownership of a blank file or directory.\"))"}],"source_content_type":"text/x-python","patch_set":12,"id":"7aaa499b_2e5becf7","line":261,"updated":"2015-04-15 16:18:17.000000000","message":"I notice that you don\u0027t trap the case where neither a user or group is passed in - it just does nothing.  Wouldn\u0027t that also indicate a coding issue?  (I saw the comment above that it\u0027s a no-op, just thinking that we should trap it too).","commit_id":"0d6e7181e97859c8c9ef1fb4834ccb543931b191"},{"author":{"_account_id":10295,"name":"mariam john","email":"mariamj@us.ibm.com","username":"mariamj"},"change_message_id":"6c8fee2f99619918d66fba2028735099358494f1","unresolved":false,"context_lines":[{"line_number":225,"context_line":"    if dir_path:"},{"line_number":226,"context_line":"        _create_directory(dir_path, force, **kwargs)"},{"line_number":227,"context_line":"        if user or group:"},{"line_number":228,"context_line":"            chown(dir_path, user, group, **kwargs)"},{"line_number":229,"context_line":"    else:"},{"line_number":230,"context_line":"        raise exception.UnprocessableEntity("},{"line_number":231,"context_line":"            _(\"Cannot create a blank directory.\"))"}],"source_content_type":"text/x-python","patch_set":15,"id":"5aa745d1_cf04df16","line":228,"updated":"2015-04-16 18:58:04.000000000","message":"Do we really need to check this condition here? If both user and group is not specified then it wont go to chown and then the exception wont be raised there. We could either raise that exception here or remove this if condition from here.","commit_id":"32b576bf2f90a9246cfa452d8904caad26cfe36a"},{"author":{"_account_id":14576,"name":"Petr Malik","email":"pmalik@tesora.com","username":"pmalik"},"change_message_id":"c678912a508dc8cde7b1ef84107d89c8e828e366","unresolved":false,"context_lines":[{"line_number":225,"context_line":"    if dir_path:"},{"line_number":226,"context_line":"        _create_directory(dir_path, force, **kwargs)"},{"line_number":227,"context_line":"        if user or group:"},{"line_number":228,"context_line":"            chown(dir_path, user, group, **kwargs)"},{"line_number":229,"context_line":"    else:"},{"line_number":230,"context_line":"        raise exception.UnprocessableEntity("},{"line_number":231,"context_line":"            _(\"Cannot create a blank directory.\"))"}],"source_content_type":"text/x-python","patch_set":15,"id":"5aa745d1_ef88bb24","line":228,"in_reply_to":"5aa745d1_cf04df16","updated":"2015-04-16 19:13:00.000000000","message":"This is not supposed to raise that exception. It\u0027s fine to call \u0027create_directory\u0027 without user/group - as opposed to \u0027chown\u0027 - the directory then belongs to the creator.","commit_id":"32b576bf2f90a9246cfa452d8904caad26cfe36a"},{"author":{"_account_id":10295,"name":"mariam john","email":"mariamj@us.ibm.com","username":"mariamj"},"change_message_id":"bdd5c09355646641bdd8782de4c1a8a82b46e990","unresolved":false,"context_lines":[{"line_number":225,"context_line":"    if dir_path:"},{"line_number":226,"context_line":"        _create_directory(dir_path, force, **kwargs)"},{"line_number":227,"context_line":"        if user or group:"},{"line_number":228,"context_line":"            chown(dir_path, user, group, **kwargs)"},{"line_number":229,"context_line":"    else:"},{"line_number":230,"context_line":"        raise exception.UnprocessableEntity("},{"line_number":231,"context_line":"            _(\"Cannot create a blank directory.\"))"}],"source_content_type":"text/x-python","patch_set":15,"id":"5aa745d1_4fd9eff2","line":228,"in_reply_to":"5aa745d1_ef88bb24","updated":"2015-04-16 19:19:38.000000000","message":"You are right. In this case, we are creating a directory. I missed that. Thanks for clarifying.","commit_id":"32b576bf2f90a9246cfa452d8904caad26cfe36a"},{"author":{"_account_id":10295,"name":"mariam john","email":"mariamj@us.ibm.com","username":"mariamj"},"change_message_id":"6c8fee2f99619918d66fba2028735099358494f1","unresolved":false,"context_lines":[{"line_number":260,"context_line":"            _(\"Cannot change ownership of a blank file or directory.\"))"},{"line_number":261,"context_line":"    if not user and not group:"},{"line_number":262,"context_line":"        raise exception.UnprocessableEntity("},{"line_number":263,"context_line":"            _(\"Missing owner and/or group.\"))"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"    owner_group_modifier \u003d _build_user_group_pair(user, group)"},{"line_number":266,"context_line":"    options \u003d ((\u0027f\u0027, force), (\u0027R\u0027, recursive))"}],"source_content_type":"text/x-python","patch_set":15,"id":"5aa745d1_ef743cd0","line":263,"updated":"2015-04-16 18:58:04.000000000","message":"Adding this check does make sense after all. Just one suggestion about the message - This should raise an error if both the user and group is missing and not \u0027or\u0027","commit_id":"32b576bf2f90a9246cfa452d8904caad26cfe36a"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"822df0290de4387a942fe2e7a3912ee82e901ebc","unresolved":false,"context_lines":[{"line_number":260,"context_line":"            _(\"Cannot change ownership of a blank file or directory.\"))"},{"line_number":261,"context_line":"    if not user and not group:"},{"line_number":262,"context_line":"        raise exception.UnprocessableEntity("},{"line_number":263,"context_line":"            _(\"Missing owner and/or group.\"))"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"    owner_group_modifier \u003d _build_user_group_pair(user, group)"},{"line_number":266,"context_line":"    options \u003d ((\u0027f\u0027, force), (\u0027R\u0027, recursive))"}],"source_content_type":"text/x-python","patch_set":15,"id":"3ab451fb_275c9524","line":263,"in_reply_to":"3ab451fb_357dde1c","updated":"2015-04-17 17:33:59.000000000","message":"OK, let me see if I get this. I believe that the code and the message don\u0027t say the same thing.\n\nif !user \u0026\u0026 !group\n\nwill be true if and only if neither are specified. The code intends that at least one must be specified.\n\nThe error message says Missing owner and/or group which doesn\u0027t tell me whether it is \"and\" or \"or\".\n\nHow about changing the message to \"Please specify either an owner or group\", or \"An owner or a group (or both) must be specified\"\n\nBut, now that I understand the intent of the code and the fact is that the only issue we have is the verbiage of the message, I will remove my objection (-1) on the review.","commit_id":"32b576bf2f90a9246cfa452d8904caad26cfe36a"},{"author":{"_account_id":10295,"name":"mariam john","email":"mariamj@us.ibm.com","username":"mariamj"},"change_message_id":"b3d1b12f05f65f39e457b384aedc63faa7a9e734","unresolved":false,"context_lines":[{"line_number":260,"context_line":"            _(\"Cannot change ownership of a blank file or directory.\"))"},{"line_number":261,"context_line":"    if not user and not group:"},{"line_number":262,"context_line":"        raise exception.UnprocessableEntity("},{"line_number":263,"context_line":"            _(\"Missing owner and/or group.\"))"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"    owner_group_modifier \u003d _build_user_group_pair(user, group)"},{"line_number":266,"context_line":"    options \u003d ((\u0027f\u0027, force), (\u0027R\u0027, recursive))"}],"source_content_type":"text/x-python","patch_set":15,"id":"3ab451fb_357dde1c","line":263,"in_reply_to":"3ab451fb_4311dcab","updated":"2015-04-17 17:10:08.000000000","message":"How aboutsaying \"Missing owner and group. Please specify owner and/or group.\"","commit_id":"32b576bf2f90a9246cfa452d8904caad26cfe36a"},{"author":{"_account_id":14576,"name":"Petr Malik","email":"pmalik@tesora.com","username":"pmalik"},"change_message_id":"1c4e4d9678abb7c4b1f482a840d865a3502ebedc","unresolved":false,"context_lines":[{"line_number":260,"context_line":"            _(\"Cannot change ownership of a blank file or directory.\"))"},{"line_number":261,"context_line":"    if not user and not group:"},{"line_number":262,"context_line":"        raise exception.UnprocessableEntity("},{"line_number":263,"context_line":"            _(\"Missing owner and/or group.\"))"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"    owner_group_modifier \u003d _build_user_group_pair(user, group)"},{"line_number":266,"context_line":"    options \u003d ((\u0027f\u0027, force), (\u0027R\u0027, recursive))"}],"source_content_type":"text/x-python","patch_set":15,"id":"3ab451fb_4311dcab","line":263,"in_reply_to":"5aa745d1_8f5ff771","updated":"2015-04-17 15:01:02.000000000","message":"Removing the \u0027or\u0027 would imply that both \u0027user\u0027 and \u0027group\u0027 have to be specified, but that\u0027s not the case.\nThe caller is required to provide either one or both.\n\nThe if condition fires only when none is given.\n\nI am open to suggestions, but I personally think that saying \u0027and/or\u0027 is pretty canonical way of saying one or both.","commit_id":"32b576bf2f90a9246cfa452d8904caad26cfe36a"},{"author":{"_account_id":10295,"name":"mariam john","email":"mariamj@us.ibm.com","username":"mariamj"},"change_message_id":"bdd5c09355646641bdd8782de4c1a8a82b46e990","unresolved":false,"context_lines":[{"line_number":260,"context_line":"            _(\"Cannot change ownership of a blank file or directory.\"))"},{"line_number":261,"context_line":"    if not user and not group:"},{"line_number":262,"context_line":"        raise exception.UnprocessableEntity("},{"line_number":263,"context_line":"            _(\"Missing owner and/or group.\"))"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"    owner_group_modifier \u003d _build_user_group_pair(user, group)"},{"line_number":266,"context_line":"    options \u003d ((\u0027f\u0027, force), (\u0027R\u0027, recursive))"}],"source_content_type":"text/x-python","patch_set":15,"id":"5aa745d1_8f5ff771","line":263,"in_reply_to":"5aa745d1_8fb937ec","updated":"2015-04-16 19:19:38.000000000","message":"Thats right but  this specific condition is checking that if both user and group is missing then raise the condition that both are missing. I agree that if either or both are present, it wouldnt raise this exception. Am I missing something?","commit_id":"32b576bf2f90a9246cfa452d8904caad26cfe36a"},{"author":{"_account_id":14576,"name":"Petr Malik","email":"pmalik@tesora.com","username":"pmalik"},"change_message_id":"c678912a508dc8cde7b1ef84107d89c8e828e366","unresolved":false,"context_lines":[{"line_number":260,"context_line":"            _(\"Cannot change ownership of a blank file or directory.\"))"},{"line_number":261,"context_line":"    if not user and not group:"},{"line_number":262,"context_line":"        raise exception.UnprocessableEntity("},{"line_number":263,"context_line":"            _(\"Missing owner and/or group.\"))"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"    owner_group_modifier \u003d _build_user_group_pair(user, group)"},{"line_number":266,"context_line":"    options \u003d ((\u0027f\u0027, force), (\u0027R\u0027, recursive))"}],"source_content_type":"text/x-python","patch_set":15,"id":"5aa745d1_8fb937ec","line":263,"in_reply_to":"5aa745d1_ef743cd0","updated":"2015-04-16 19:13:00.000000000","message":"It\u0027s valid to specify just owner, just group or both, hence \u0027and/or\u0027.","commit_id":"32b576bf2f90a9246cfa452d8904caad26cfe36a"}],"trove/guestagent/datastore/experimental/couchdb/service.py":[{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"b676ed2a3f37d3e33b19106d1f4b66198a74837c","unresolved":false,"context_lines":[{"line_number":79,"context_line":"        try:"},{"line_number":80,"context_line":"            LOG.debug(\"Changing permissions.\")"},{"line_number":81,"context_line":"            operating_system.chown("},{"line_number":82,"context_line":"                \u0027couchdb\u0027, \u0027couchdb\u0027, COUCHDB_LIB_DIR, as_root\u003dTrue"},{"line_number":83,"context_line":"            )"},{"line_number":84,"context_line":"            operating_system.chown("},{"line_number":85,"context_line":"                \u0027couchdb\u0027, \u0027couchdb\u0027, COUCHDB_LOG_DIR, as_root\u003dTrue"}],"source_content_type":"text/x-python","patch_set":16,"id":"bab921fb_1b21e9fb","line":82,"updated":"2015-04-23 18:26:11.000000000","message":"COUCH_LIB_DIR needs to come before \u0027couchdb\u0027","commit_id":"9d912802fdd4642630b3ff4064a4f15730cea75c"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"b676ed2a3f37d3e33b19106d1f4b66198a74837c","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                \u0027couchdb\u0027, \u0027couchdb\u0027, COUCHDB_LIB_DIR, as_root\u003dTrue"},{"line_number":83,"context_line":"            )"},{"line_number":84,"context_line":"            operating_system.chown("},{"line_number":85,"context_line":"                \u0027couchdb\u0027, \u0027couchdb\u0027, COUCHDB_LOG_DIR, as_root\u003dTrue"},{"line_number":86,"context_line":"            )"},{"line_number":87,"context_line":"            operating_system.chown("},{"line_number":88,"context_line":"                \u0027couchdb\u0027, \u0027couchdb\u0027, COUCHDB_BIN_DIR, as_root\u003dTrue"}],"source_content_type":"text/x-python","patch_set":16,"id":"bab921fb_db7931fe","line":85,"updated":"2015-04-23 18:26:11.000000000","message":"ditto","commit_id":"9d912802fdd4642630b3ff4064a4f15730cea75c"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"b676ed2a3f37d3e33b19106d1f4b66198a74837c","unresolved":false,"context_lines":[{"line_number":85,"context_line":"                \u0027couchdb\u0027, \u0027couchdb\u0027, COUCHDB_LOG_DIR, as_root\u003dTrue"},{"line_number":86,"context_line":"            )"},{"line_number":87,"context_line":"            operating_system.chown("},{"line_number":88,"context_line":"                \u0027couchdb\u0027, \u0027couchdb\u0027, COUCHDB_BIN_DIR, as_root\u003dTrue"},{"line_number":89,"context_line":"            )"},{"line_number":90,"context_line":"            operating_system.chown("},{"line_number":91,"context_line":"                \u0027couchdb\u0027, \u0027couchdb\u0027, COUCHDB_CONFIG_DIR, as_root\u003dTrue"}],"source_content_type":"text/x-python","patch_set":16,"id":"bab921fb_3b6b8d20","line":88,"updated":"2015-04-23 18:26:11.000000000","message":"ditto","commit_id":"9d912802fdd4642630b3ff4064a4f15730cea75c"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"b676ed2a3f37d3e33b19106d1f4b66198a74837c","unresolved":false,"context_lines":[{"line_number":88,"context_line":"                \u0027couchdb\u0027, \u0027couchdb\u0027, COUCHDB_BIN_DIR, as_root\u003dTrue"},{"line_number":89,"context_line":"            )"},{"line_number":90,"context_line":"            operating_system.chown("},{"line_number":91,"context_line":"                \u0027couchdb\u0027, \u0027couchdb\u0027, COUCHDB_CONFIG_DIR, as_root\u003dTrue"},{"line_number":92,"context_line":"            )"},{"line_number":93,"context_line":"            operating_system.chmod(COUCHDB_LIB_DIR, FileMode.ADD_GRP_RW,"},{"line_number":94,"context_line":"                                   as_root\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":16,"id":"bab921fb_fb74f5fd","line":91,"updated":"2015-04-23 18:26:11.000000000","message":"ditto","commit_id":"9d912802fdd4642630b3ff4064a4f15730cea75c"}],"trove/tests/unittests/guestagent/test_dbaas.py":[{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"2ded2bb6190e5892022ba3dcbe26ac7bc9d58720","unresolved":false,"context_lines":[{"line_number":1625,"context_line":"        configuration \u003d \u0027some arbitrary configuration text\u0027"},{"line_number":1626,"context_line":""},{"line_number":1627,"context_line":"        with patch.object(utils, \u0027execute_with_timeout\u0027,"},{"line_number":1628,"context_line":"                          return_value\u003dTrue) as exec_call:"},{"line_number":1629,"context_line":"            self.cassandra.write_config(configuration,"},{"line_number":1630,"context_line":"                                        execute_function\u003dexec_call,"},{"line_number":1631,"context_line":"                                        mkstemp_function\u003dmock_mkstemp)"}],"source_content_type":"text/x-python","patch_set":2,"id":"da9b358b_84e1e400","line":1628,"updated":"2015-03-31 18:27:35.000000000","message":"You don\u0027t need to patch execute_with_timeout here, as write_config has been \u0027fixed\u0027 to take an execute fn passed in.  I think you mean to patch operating_system.update_owner","commit_id":"a20b71c2f24bb370214ded14203861c362e5bbaa"}],"trove/tests/unittests/guestagent/test_operating_system.py":[{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"2ded2bb6190e5892022ba3dcbe26ac7bc9d58720","unresolved":false,"context_lines":[{"line_number":21,"context_line":"from trove.guestagent.common import operating_system"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"class Test(testtools.TestCase):"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    def test_update_owner(self):"},{"line_number":27,"context_line":"        with patch.object(utils, \u0027execute_with_timeout\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"da9b358b_092ccda9","line":24,"updated":"2015-03-31 18:27:35.000000000","message":"There seems to be a lot of redundancy here.  Could this be consolidated somehow?","commit_id":"a20b71c2f24bb370214ded14203861c362e5bbaa"}],"trove/tests/unittests/guestagent/test_redis_manager.py":[{"author":{"_account_id":7806,"name":"Sushil Kumar","email":"skm.net@gmail.com","username":"sushilkm"},"change_message_id":"b0825a8ea865d4774ba04d2d55deb176075c9fbf","unresolved":false,"context_lines":[{"line_number":82,"context_line":"        redis_service.RedisApp.start_redis \u003d MagicMock(return_value\u003dNone)"},{"line_number":83,"context_line":"        redis_service.RedisApp.install_if_needed \u003d MagicMock(return_value\u003dNone)"},{"line_number":84,"context_line":"        redis_service.RedisApp.write_config \u003d MagicMock(return_value\u003dNone)"},{"line_number":85,"context_line":"        operating_system.chown \u003d MagicMock(return_value\u003dNone)"},{"line_number":86,"context_line":"        redis_service.RedisApp.restart \u003d MagicMock(return_value\u003dNone)"},{"line_number":87,"context_line":"        mock_status.begin_install \u003d MagicMock(return_value\u003dNone)"},{"line_number":88,"context_line":"        VolumeDevice.format \u003d MagicMock(return_value\u003dNone)"}],"source_content_type":"text/x-python","patch_set":19,"id":"3acd31a7_88ed9cf1","line":85,"updated":"2015-05-01 15:30:35.000000000","message":"even this one is also not unmocked :(","commit_id":"0397f9c6a00c200572a55ba95d8a92a7a9596168"}],"trove/tests/unittests/guestagent/test_volume.py":[{"author":{"_account_id":7806,"name":"Sushil Kumar","email":"skm.net@gmail.com","username":"sushilkm"},"change_message_id":"d74b35a6a8d888f3e52e77769cc0f797c4e116ac","unresolved":false,"context_lines":[{"line_number":207,"context_line":"        os.path.exists \u003d MagicMock(return_value\u003dFalse)"},{"line_number":208,"context_line":"        fake_spawn \u003d _setUp_fake_spawn()"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"        utils.execute_with_timeout \u003d Mock()"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"        self.volumeMountPoint.mount()"},{"line_number":213,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"3acd31a7_2db7ba87","line":210,"updated":"2015-05-01 15:10:38.000000000","message":"there is no unmocking of this utils.execute_with_timeout.\n\nthis is unsafe.","commit_id":"0397f9c6a00c200572a55ba95d8a92a7a9596168"}]}