)]}'
{"trove/guestagent/datastore/experimental/vertica/service.py":[{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"8c61c3587cb49ac908c9cea8e4168a33b9bc1a4b","unresolved":false,"context_lines":[{"line_number":197,"context_line":"                LOG.debug(\"Adding the %s library as %s.\" %"},{"line_number":198,"context_line":"                          (func_name, lib_name))"},{"line_number":199,"context_line":"                try:"},{"line_number":200,"context_line":"                    out, err \u003d system.exec_vsql_command("},{"line_number":201,"context_line":"                        password,"},{"line_number":202,"context_line":"                        system.CREATE_LIBRARY % (lib_name, path)"},{"line_number":203,"context_line":"                    )"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa69d971_404a2757","line":200,"updated":"2016-01-07 14:54:23.000000000","message":"so this is frowned upon. You should consider using oslo_concurrency.processutils.\n\nNow that I see what the rest of this module is doing, maybe just enter a bug about this as that change would be out of scope of your present change.","commit_id":"845d910fcebf3236308782fdcd74aa5d5b69cb36"},{"author":{"_account_id":15321,"name":"Matthew Van Dijk","email":"mvandijk@tesora.com","username":"mvandijk"},"change_message_id":"ef773af4918e5ba35dd7a7bfc1febe50890ece39","unresolved":false,"context_lines":[{"line_number":197,"context_line":"                LOG.debug(\"Adding the %s library as %s.\" %"},{"line_number":198,"context_line":"                          (func_name, lib_name))"},{"line_number":199,"context_line":"                try:"},{"line_number":200,"context_line":"                    out, err \u003d system.exec_vsql_command("},{"line_number":201,"context_line":"                        password,"},{"line_number":202,"context_line":"                        system.CREATE_LIBRARY % (lib_name, path)"},{"line_number":203,"context_line":"                    )"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa69d971_200933de","line":200,"in_reply_to":"fa69d971_404a2757","updated":"2016-01-07 14:57:46.000000000","message":"Just keeping it consistent with the rest of the module.","commit_id":"845d910fcebf3236308782fdcd74aa5d5b69cb36"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"8c61c3587cb49ac908c9cea8e4168a33b9bc1a4b","unresolved":false,"context_lines":[{"line_number":202,"context_line":"                        system.CREATE_LIBRARY % (lib_name, path)"},{"line_number":203,"context_line":"                    )"},{"line_number":204,"context_line":"                    if err:"},{"line_number":205,"context_line":"                        LOG.error(err)"},{"line_number":206,"context_line":"                        raise RuntimeError(_(\"Failed to create library %s.\")"},{"line_number":207,"context_line":"                                           % lib_name)"},{"line_number":208,"context_line":"                    out, err \u003d system.exec_vsql_command("}],"source_content_type":"text/x-python","patch_set":1,"id":"fa69d971_25670527","line":205,"updated":"2016-01-07 14:54:23.000000000","message":"Is it sufficient to log just err, often times out and err together tell the full story.\n\nAlso, I don\u0027t know of too many situations where you get err but an exception is not thrown. I guess that in theory a command could return exit code of 0 and send stuff to stderr but that\u0027s kind of weird.\n\nAnd since you are using system.execute or whatever, passwords and stuff will/could leak.","commit_id":"845d910fcebf3236308782fdcd74aa5d5b69cb36"},{"author":{"_account_id":15321,"name":"Matthew Van Dijk","email":"mvandijk@tesora.com","username":"mvandijk"},"change_message_id":"ef773af4918e5ba35dd7a7bfc1febe50890ece39","unresolved":false,"context_lines":[{"line_number":202,"context_line":"                        system.CREATE_LIBRARY % (lib_name, path)"},{"line_number":203,"context_line":"                    )"},{"line_number":204,"context_line":"                    if err:"},{"line_number":205,"context_line":"                        LOG.error(err)"},{"line_number":206,"context_line":"                        raise RuntimeError(_(\"Failed to create library %s.\")"},{"line_number":207,"context_line":"                                           % lib_name)"},{"line_number":208,"context_line":"                    out, err \u003d system.exec_vsql_command("}],"source_content_type":"text/x-python","patch_set":1,"id":"fa69d971_c00db7e9","line":205,"in_reply_to":"fa69d971_25670527","updated":"2016-01-07 14:57:46.000000000","message":"Again, just keeping it consistent with the rest of the module.","commit_id":"845d910fcebf3236308782fdcd74aa5d5b69cb36"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"8c61c3587cb49ac908c9cea8e4168a33b9bc1a4b","unresolved":false,"context_lines":[{"line_number":215,"context_line":"                        raise RuntimeError(_(\"Failed to create source %s.\")"},{"line_number":216,"context_line":"                                           % func_name)"},{"line_number":217,"context_line":"                except RuntimeError:"},{"line_number":218,"context_line":"                    continue"},{"line_number":219,"context_line":"                loaded_udls.append(func_name)"},{"line_number":220,"context_line":"            else:"},{"line_number":221,"context_line":"                LOG.debug(\"Skipping %s as path %s not found.\" %"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa69d971_e5156d49","line":218,"updated":"2016-01-07 14:54:23.000000000","message":"so it is OK to load some but not all libraries?","commit_id":"845d910fcebf3236308782fdcd74aa5d5b69cb36"},{"author":{"_account_id":15321,"name":"Matthew Van Dijk","email":"mvandijk@tesora.com","username":"mvandijk"},"change_message_id":"72fed24bfd60bc8ab87ddd50442f8691ffece152","unresolved":false,"context_lines":[{"line_number":215,"context_line":"                        raise RuntimeError(_(\"Failed to create source %s.\")"},{"line_number":216,"context_line":"                                           % func_name)"},{"line_number":217,"context_line":"                except RuntimeError:"},{"line_number":218,"context_line":"                    continue"},{"line_number":219,"context_line":"                loaded_udls.append(func_name)"},{"line_number":220,"context_line":"            else:"},{"line_number":221,"context_line":"                LOG.debug(\"Skipping %s as path %s not found.\" %"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa69d971_b0498ed2","line":218,"in_reply_to":"fa69d971_263a9ae8","updated":"2016-01-08 19:01:22.000000000","message":"@Peter - Right. But I also intend it to keep trying to load whatever it can.","commit_id":"845d910fcebf3236308782fdcd74aa5d5b69cb36"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"387bb76b6cad02ccca40868a2b3c502d88b703a1","unresolved":false,"context_lines":[{"line_number":215,"context_line":"                        raise RuntimeError(_(\"Failed to create source %s.\")"},{"line_number":216,"context_line":"                                           % func_name)"},{"line_number":217,"context_line":"                except RuntimeError:"},{"line_number":218,"context_line":"                    continue"},{"line_number":219,"context_line":"                loaded_udls.append(func_name)"},{"line_number":220,"context_line":"            else:"},{"line_number":221,"context_line":"                LOG.debug(\"Skipping %s as path %s not found.\" %"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa69d971_303b5eed","line":218,"in_reply_to":"fa69d971_b0498ed2","updated":"2016-01-08 19:03:29.000000000","message":"Yep, looks good from a code point of view (Amrith\u0027s concerns notwithstanding for the moment).","commit_id":"845d910fcebf3236308782fdcd74aa5d5b69cb36"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"34f94b6df9b3e8a3801e78df96a674557fc70073","unresolved":false,"context_lines":[{"line_number":215,"context_line":"                        raise RuntimeError(_(\"Failed to create source %s.\")"},{"line_number":216,"context_line":"                                           % func_name)"},{"line_number":217,"context_line":"                except RuntimeError:"},{"line_number":218,"context_line":"                    continue"},{"line_number":219,"context_line":"                loaded_udls.append(func_name)"},{"line_number":220,"context_line":"            else:"},{"line_number":221,"context_line":"                LOG.debug(\"Skipping %s as path %s not found.\" %"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa69d971_263a9ae8","line":218,"in_reply_to":"fa69d971_c024d761","updated":"2016-01-07 22:10:44.000000000","message":"It won\u0027t - nothing in post_prepare can cause it to go into error state (exceptions are caught and logged, but not propagated).","commit_id":"845d910fcebf3236308782fdcd74aa5d5b69cb36"},{"author":{"_account_id":15321,"name":"Matthew Van Dijk","email":"mvandijk@tesora.com","username":"mvandijk"},"change_message_id":"ef773af4918e5ba35dd7a7bfc1febe50890ece39","unresolved":false,"context_lines":[{"line_number":215,"context_line":"                        raise RuntimeError(_(\"Failed to create source %s.\")"},{"line_number":216,"context_line":"                                           % func_name)"},{"line_number":217,"context_line":"                except RuntimeError:"},{"line_number":218,"context_line":"                    continue"},{"line_number":219,"context_line":"                loaded_udls.append(func_name)"},{"line_number":220,"context_line":"            else:"},{"line_number":221,"context_line":"                LOG.debug(\"Skipping %s as path %s not found.\" %"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa69d971_c024d761","line":218,"in_reply_to":"fa69d971_e5156d49","updated":"2016-01-07 14:57:46.000000000","message":"Yes I didn\u0027t want this to crash the prepare sequence if the base image didn\u0027t have the libraries available.","commit_id":"845d910fcebf3236308782fdcd74aa5d5b69cb36"},{"author":{"_account_id":14576,"name":"Petr Malik","email":"pmalik@tesora.com","username":"pmalik"},"change_message_id":"a1f135650b1170406016972cab3c3c9b80ace4dc","unresolved":false,"context_lines":[{"line_number":215,"context_line":"                        raise RuntimeError(_(\"Failed to create source %s.\")"},{"line_number":216,"context_line":"                                           % func_name)"},{"line_number":217,"context_line":"                except RuntimeError:"},{"line_number":218,"context_line":"                    continue"},{"line_number":219,"context_line":"                loaded_udls.append(func_name)"},{"line_number":220,"context_line":"            else:"},{"line_number":221,"context_line":"                LOG.debug(\"Skipping %s as path %s not found.\" %"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a68dd71_07e9cdb6","line":218,"updated":"2016-01-25 17:35:59.000000000","message":"As below, we may want to make it clear that the lib is going to be skipped.","commit_id":"a637ec0d5dfa9ff053c3d44b7566b8056c54fdbf"},{"author":{"_account_id":9782,"name":"Morgan Jones","email":"morgan@tesora.com","username":"vgnbkr"},"change_message_id":"7193b65a20509f1b10fd2fcb688cf99fb245550b","unresolved":false,"context_lines":[{"line_number":215,"context_line":"                        raise RuntimeError(_(\"Failed to create source %s.\")"},{"line_number":216,"context_line":"                                           % func_name)"},{"line_number":217,"context_line":"                except RuntimeError:"},{"line_number":218,"context_line":"                    continue"},{"line_number":219,"context_line":"                loaded_udls.append(func_name)"},{"line_number":220,"context_line":"            else:"},{"line_number":221,"context_line":"                LOG.debug(\"Skipping %s as path %s not found.\" %"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a68dd71_acc3cb37","line":218,"in_reply_to":"9a68dd71_07e9cdb6","updated":"2016-01-26 17:03:47.000000000","message":"For each library defined in the Trove system module, the path is checked above to ensure that the library is indeed installed on the system.  That being the case, if the library fails to load the instance should go into error state.","commit_id":"a637ec0d5dfa9ff053c3d44b7566b8056c54fdbf"},{"author":{"_account_id":14576,"name":"Petr Malik","email":"pmalik@tesora.com","username":"pmalik"},"change_message_id":"a1f135650b1170406016972cab3c3c9b80ace4dc","unresolved":false,"context_lines":[{"line_number":218,"context_line":"                    continue"},{"line_number":219,"context_line":"                loaded_udls.append(func_name)"},{"line_number":220,"context_line":"            else:"},{"line_number":221,"context_line":"                LOG.debug(\"Skipping %s as path %s not found.\" %"},{"line_number":222,"context_line":"                          (func_name, path))"},{"line_number":223,"context_line":"        LOG.info(_(\"The following UDL functions are available for use: %s\")"},{"line_number":224,"context_line":"                 % loaded_udls)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a68dd71_a7a6c1ab","line":221,"updated":"2016-01-25 17:35:59.000000000","message":"This should probably be a warn() message at least - the end users normally won\u0027t see any debug messages. Otherwise, they may be wondering why some libs did not get loaded.","commit_id":"a637ec0d5dfa9ff053c3d44b7566b8056c54fdbf"},{"author":{"_account_id":9782,"name":"Morgan Jones","email":"morgan@tesora.com","username":"vgnbkr"},"change_message_id":"26d497ea49bdade0b1dd0e15914071448af7da34","unresolved":false,"context_lines":[{"line_number":215,"context_line":"                                       % func_name)"},{"line_number":216,"context_line":"                loaded_udls.append(func_name)"},{"line_number":217,"context_line":"            else:"},{"line_number":218,"context_line":"                LOG.debug(\"Skipping %s as path %s not found.\" %"},{"line_number":219,"context_line":"                          (func_name, path))"},{"line_number":220,"context_line":"        LOG.info(_(\"The following UDL functions are available for use: %s\")"},{"line_number":221,"context_line":"                 % loaded_udls)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a68dd71_d8d7cb21","line":218,"updated":"2016-01-26 22:40:20.000000000","message":"I think you missed Petr\u0027s comment that this should be a WARN, which I agree with.","commit_id":"56388056fa6c9fa3cb6585153f9b3b4d58a134f1"}],"trove/tests/unittests/guestagent/test_vertica_manager.py":[{"author":{"_account_id":9782,"name":"Morgan Jones","email":"morgan@tesora.com","username":"vgnbkr"},"change_message_id":"7193b65a20509f1b10fd2fcb688cf99fb245550b","unresolved":false,"context_lines":[{"line_number":110,"context_line":"        VerticaApp.install_vertica \u003d MagicMock(return_value\u003dNone)"},{"line_number":111,"context_line":"        VerticaApp.create_db \u003d MagicMock(return_value\u003dNone)"},{"line_number":112,"context_line":"        VerticaApp.prepare_for_install_vertica \u003d MagicMock(return_value\u003dNone)"},{"line_number":113,"context_line":"        VerticaApp.add_udls \u003d MagicMock()"},{"line_number":114,"context_line":"        # invocation"},{"line_number":115,"context_line":"        self.manager.prepare(context\u003dself.context, packages\u003dpackages,"},{"line_number":116,"context_line":"                             config_contents\u003dconfig_content,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a68dd71_ace42bac","line":113,"updated":"2016-01-26 17:03:47.000000000","message":"I don\u0027t think this is sufficient testing of this feature.  At a minimum, you should try to test that the appropriate execute calls get called (i.e., go a level deeper into the service module)","commit_id":"a637ec0d5dfa9ff053c3d44b7566b8056c54fdbf"}]}