)]}'
{"trove/guestagent/datastore/mysql/manager.py":[{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"962bc1c6b8eaccaf66633648009a57440f727067","unresolved":false,"context_lines":[{"line_number":210,"context_line":"        if overrides:"},{"line_number":211,"context_line":"            app.update_overrides(overrides)"},{"line_number":212,"context_line":"        else:"},{"line_number":213,"context_line":"            app.remove_overrides()"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":"    def apply_overrides(self, context, overrides):"},{"line_number":216,"context_line":"        LOG.debug(\"Applying overrides (%s).\" % overrides)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_e40e373a","line":213,"updated":"2015-03-10 04:31:30.000000000","message":"Hmm, the \u0027remove\u0027 kwarg isn\u0027t being used anymore","commit_id":"9c0acbef11aa00abb9120ea8d122a430a23d44cf"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"5034009c95d6af0cfa29b4e44a0285d0286d8336","unresolved":false,"context_lines":[{"line_number":210,"context_line":"        if overrides:"},{"line_number":211,"context_line":"            app.update_overrides(overrides)"},{"line_number":212,"context_line":"        else:"},{"line_number":213,"context_line":"            app.remove_overrides()"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":"    def apply_overrides(self, context, overrides):"},{"line_number":216,"context_line":"        LOG.debug(\"Applying overrides (%s).\" % overrides)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_678792d8","line":213,"in_reply_to":"9a80dd14_58d520e3","updated":"2015-03-10 14:11:33.000000000","message":"Sure, but doesn\u0027t ignoring \u0027remove\u003dFalse\u0027 also break the contract?  It isn\u0027t just about the signature; the behaviour also needs to be the same, right?","commit_id":"9c0acbef11aa00abb9120ea8d122a430a23d44cf"},{"author":{"_account_id":14829,"name":"Ashleigh Farnham","username":"ashleighfarnham","inactive":true},"change_message_id":"5b31b300488f409d27dd1eed6db83ed20572a631","unresolved":false,"context_lines":[{"line_number":210,"context_line":"        if overrides:"},{"line_number":211,"context_line":"            app.update_overrides(overrides)"},{"line_number":212,"context_line":"        else:"},{"line_number":213,"context_line":"            app.remove_overrides()"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":"    def apply_overrides(self, context, overrides):"},{"line_number":216,"context_line":"        LOG.debug(\"Applying overrides (%s).\" % overrides)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_d059cd5c","line":213,"in_reply_to":"9a80dd14_678792d8","updated":"2015-03-10 16:27:47.000000000","message":"The existing behavior is wrong and breaks mysql. The param should never have been there to begin with. Even if this does break the implied contract that the param be used I don\u0027t think that should stop us ensuring correct, sane behavior. I agree changing the contract by ignoring the param isn\u0027t the ideal. We could also change this to an elif but I really don\u0027t think that will make any difference to the fundamental change here.","commit_id":"9c0acbef11aa00abb9120ea8d122a430a23d44cf"},{"author":{"_account_id":13355,"name":"Edmond Kotowski","email":"ekotowski@gmail.com","username":"ekotowski"},"change_message_id":"188ef6a8037c4191fd35aec4ac7310d7fe18d5ed","unresolved":false,"context_lines":[{"line_number":210,"context_line":"        if overrides:"},{"line_number":211,"context_line":"            app.update_overrides(overrides)"},{"line_number":212,"context_line":"        else:"},{"line_number":213,"context_line":"            app.remove_overrides()"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":"    def apply_overrides(self, context, overrides):"},{"line_number":216,"context_line":"        LOG.debug(\"Applying overrides (%s).\" % overrides)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_58d520e3","line":213,"in_reply_to":"9a80dd14_e40e373a","updated":"2015-03-10 07:02:04.000000000","message":"Peter this cannot be changed. We dove into the source on this. This has to do with breaking the signature for the RPC call. The remove parameter is not needed and was an incorrect implementation in the first place.","commit_id":"9c0acbef11aa00abb9120ea8d122a430a23d44cf"},{"author":{"_account_id":14829,"name":"Ashleigh Farnham","username":"ashleighfarnham","inactive":true},"change_message_id":"2f04ac4de8da6c39cda121c884b5d610c174cfc4","unresolved":false,"context_lines":[{"line_number":210,"context_line":"        if overrides:"},{"line_number":211,"context_line":"            app.update_overrides(overrides)"},{"line_number":212,"context_line":"        else:"},{"line_number":213,"context_line":"            app.remove_overrides()"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":"    def apply_overrides(self, context, overrides):"},{"line_number":216,"context_line":"        LOG.debug(\"Applying overrides (%s).\" % overrides)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_b87b7c79","line":213,"in_reply_to":"9a80dd14_e40e373a","updated":"2015-03-10 06:42:15.000000000","message":"This is intentional. The method signature is dictated by the contract between the task manager and the guest. To remove the arg would break compatibility with older versions of the guest agent or task manager.","commit_id":"9c0acbef11aa00abb9120ea8d122a430a23d44cf"},{"author":{"_account_id":14576,"name":"Petr Malik","email":"pmalik@tesora.com","username":"pmalik"},"change_message_id":"a17d84ec4e445754ebfcb8121520414c4b66078e","unresolved":false,"context_lines":[{"line_number":205,"context_line":"        LOG.debug(\"Resized the filesystem %s.\" % mount_point)"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    def update_overrides(self, context, overrides, remove\u003dFalse):"},{"line_number":208,"context_line":"        LOG.debug(\"Updating overrides (%s).\" % overrides)"},{"line_number":209,"context_line":"        app \u003d MySqlApp(MySqlAppStatus.get())"},{"line_number":210,"context_line":"        if remove:"},{"line_number":211,"context_line":"            app.remove_overrides()"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a80dd14_efa959de","line":208,"updated":"2015-03-11 16:11:49.000000000","message":"This log message may be misleading if overrides\u003dNone AND remove\u003dFalse when nothing really happens.\nAlso both called functions in this method already do their logging.","commit_id":"e19b83295442bded0c39c37fdcfb531ecb334d38"},{"author":{"_account_id":14576,"name":"Petr Malik","email":"pmalik@tesora.com","username":"pmalik"},"change_message_id":"a17d84ec4e445754ebfcb8121520414c4b66078e","unresolved":false,"context_lines":[{"line_number":209,"context_line":"        app \u003d MySqlApp(MySqlAppStatus.get())"},{"line_number":210,"context_line":"        if remove:"},{"line_number":211,"context_line":"            app.remove_overrides()"},{"line_number":212,"context_line":"        if overrides:"},{"line_number":213,"context_line":"            app.update_overrides(overrides)"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":"    def apply_overrides(self, context, overrides):"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a80dd14_c35f1374","line":212,"updated":"2015-03-11 16:11:49.000000000","message":"\u0027update_overrides\u0027 documentation says that it writes the file only if there is any content to save. This check therefore seems unnecessary.","commit_id":"e19b83295442bded0c39c37fdcfb531ecb334d38"},{"author":{"_account_id":15090,"name":"Elvis Teixeira","email":"elvis.teixeira@lsbd.ufc.br","username":"ElvisTeixeira"},"change_message_id":"53c0e94dcc72f2d051ec16359d23a68d88e00d04","unresolved":false,"context_lines":[{"line_number":203,"context_line":"        device.resize_fs(mount_point)"},{"line_number":204,"context_line":"        LOG.debug(\"Resized the filesystem %s.\" % mount_point)"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"    def update_overrides(self, context, overrides, remove\u003dFalse):"},{"line_number":207,"context_line":"        app \u003d MySqlApp(MySqlAppStatus.get())"},{"line_number":208,"context_line":"        if remove:"},{"line_number":209,"context_line":"            app.remove_overrides()"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a80dd14_53fc23b5","line":206,"updated":"2015-03-16 11:38:44.000000000","message":"Why supress the LOG?","commit_id":"0a70dc0116401ad9266ccd2034969c850a3d98e6"},{"author":{"_account_id":14829,"name":"Ashleigh Farnham","username":"ashleighfarnham","inactive":true},"change_message_id":"8bcc43af8347f498d749fa8182b88a3ebaa56423","unresolved":false,"context_lines":[{"line_number":203,"context_line":"        device.resize_fs(mount_point)"},{"line_number":204,"context_line":"        LOG.debug(\"Resized the filesystem %s.\" % mount_point)"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"    def update_overrides(self, context, overrides, remove\u003dFalse):"},{"line_number":207,"context_line":"        app \u003d MySqlApp(MySqlAppStatus.get())"},{"line_number":208,"context_line":"        if remove:"},{"line_number":209,"context_line":"            app.remove_overrides()"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a80dd14_4c2b9881","line":206,"in_reply_to":"9a80dd14_53fc23b5","updated":"2015-03-16 15:28:00.000000000","message":"The log was ambiguous and both remove_overrides() and update_overrides() log what is happening.","commit_id":"0a70dc0116401ad9266ccd2034969c850a3d98e6"}],"trove/guestagent/datastore/mysql/service.py":[{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"babe6aaeb4287ed5202b232186fd9f2c4b16a77c","unresolved":false,"context_lines":[{"line_number":724,"context_line":"        if overrides_file:"},{"line_number":725,"context_line":"            LOG.debug(\"Writing new overrides.cnf config file.\")"},{"line_number":726,"context_line":"            self._write_config_overrides(overrides_file)"},{"line_number":727,"context_line":"        elif remove:"},{"line_number":728,"context_line":"            LOG.debug(\"Removing overrides.cnf config file.\")"},{"line_number":729,"context_line":"            self._remove_overrides()"},{"line_number":730,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9a80dd14_468023c0","line":727,"updated":"2015-03-07 18:46:43.000000000","message":"Wouldn\u0027t it make more sense to pull the \u0027remove_overrides\u0027 into its own method?  Having a method sometimes update and sometimes delete seems a bit sketchy.  (I know you didn\u0027t write the method, but the fact that you noticed it wasn\u0027t behaving the way you expected implies a refactor might be in order).  Just a suggestion.","commit_id":"267ad887aa9e02da2f760536c9ca2212c8d50520"},{"author":{"_account_id":13355,"name":"Edmond Kotowski","email":"ekotowski@gmail.com","username":"ekotowski"},"change_message_id":"a4b89e93b0003779123ec41f94f5865021a3afb2","unresolved":false,"context_lines":[{"line_number":724,"context_line":"        if overrides_file:"},{"line_number":725,"context_line":"            LOG.debug(\"Writing new overrides.cnf config file.\")"},{"line_number":726,"context_line":"            self._write_config_overrides(overrides_file)"},{"line_number":727,"context_line":"        elif remove:"},{"line_number":728,"context_line":"            LOG.debug(\"Removing overrides.cnf config file.\")"},{"line_number":729,"context_line":"            self._remove_overrides()"},{"line_number":730,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9a80dd14_ba81e7c4","line":727,"in_reply_to":"9a80dd14_468023c0","updated":"2015-03-08 23:11:31.000000000","message":"I agree that it would be better to create the remove_overrides method and  replace all the places were they are calling updates_overrides(remove\u003dTrue).","commit_id":"267ad887aa9e02da2f760536c9ca2212c8d50520"},{"author":{"_account_id":13355,"name":"Edmond Kotowski","email":"ekotowski@gmail.com","username":"ekotowski"},"change_message_id":"4f8964cf53727dcbff284b6634fbc9c6808b4fc0","unresolved":false,"context_lines":[{"line_number":732,"context_line":"        \"\"\""},{"line_number":733,"context_line":""},{"line_number":734,"context_line":"        LOG.debug(\"Removing overrides.cnf config file.\")"},{"line_number":735,"context_line":"        self._remove_overrides()"},{"line_number":736,"context_line":""},{"line_number":737,"context_line":"    def apply_overrides(self, overrides):"},{"line_number":738,"context_line":"        LOG.debug(\"Applying overrides to MySQL.\")"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_93c78d63","line":735,"updated":"2015-03-09 20:55:55.000000000","message":"At this point why not just rename _remove_overrides to remove_overrides? This pass thru method doesn\u0027t add any value considering it\u0027s only one line basically. Not sure why _remove_overrides would need to be private.","commit_id":"3621240c82339788e97a9b6222b37d0b1b59495d"},{"author":{"_account_id":14576,"name":"Petr Malik","email":"pmalik@tesora.com","username":"pmalik"},"change_message_id":"a17d84ec4e445754ebfcb8121520414c4b66078e","unresolved":false,"context_lines":[{"line_number":711,"context_line":"        finally:"},{"line_number":712,"context_line":"            self.status.end_install_or_restart()"},{"line_number":713,"context_line":""},{"line_number":714,"context_line":"    def update_overrides(self, overrides_file):"},{"line_number":715,"context_line":"        \"\"\""},{"line_number":716,"context_line":"        This function will update the MySQL overrides.cnf file"},{"line_number":717,"context_line":"        if there is content to write."}],"source_content_type":"text/x-python","patch_set":5,"id":"9a80dd14_2340ef55","line":714,"range":{"start_line":714,"start_character":31,"end_line":714,"end_character":45},"updated":"2015-03-11 16:11:49.000000000","message":"I\u0027d suggest renaming the \u0027overrides_file\u0027 variable to something like \u0027overrides\u0027 or \u0027overrideValues\u0027.\nThe current name suggests the parameter is a pointer or path to the \u0027overrides.cnf\u0027 file which is not the case as can be seen from the \u0027_write_config_overrides\u0027 function and its signature.","commit_id":"e19b83295442bded0c39c37fdcfb531ecb334d38"}]}