)]}'
{"specs/mitaka/datastore-manager-refactor.rst":[{"author":{"_account_id":14576,"name":"Petr Malik","email":"pmalik@tesora.com","username":"pmalik"},"change_message_id":"f9c7c624294153cca4902ff0b098623cf925d761","unresolved":false,"context_lines":[{"line_number":116,"context_line":"                 +-- manager.py"},{"line_number":117,"context_line":"                 +-- service.py"},{"line_number":118,"context_line":"                 +-- system.py"},{"line_number":119,"context_line":"            +-- mysql_common        \u003c-- new module"},{"line_number":120,"context_line":"             +-- __init__.py"},{"line_number":121,"context_line":"             +-- manager.py      \u003c-- renamed from mysql/manager_base.py"},{"line_number":122,"context_line":"             +-- service.py      \u003c-- renamed from mysql/service_base.py"}],"source_content_type":"text/x-rst","patch_set":1,"id":"7a2fa921_71943342","line":119,"range":{"start_line":119,"start_character":0,"end_line":119,"end_character":18},"updated":"2015-10-08 16:52:36.000000000","message":"Just a design point for future discussion.\nI still think that the \u0027mysql\u0027 manager itself could be the *common* base for other mysql-derivatives - we would have the \u0027mysql\u0027 manager and others (maria, percona) would be just extending that.","commit_id":"c1e63979d7805c88e7dfcd763e667791c5491c34"},{"author":{"_account_id":10440,"name":"Alex Tomic","email":"atomic777@gmail.com","username":"atomic77"},"change_message_id":"b820def4ba54ee89b6acf292591c65eea4f0e349","unresolved":false,"context_lines":[{"line_number":118,"context_line":"                 +-- system.py"},{"line_number":119,"context_line":"            +-- mysql_common        \u003c-- new module"},{"line_number":120,"context_line":"             +-- __init__.py"},{"line_number":121,"context_line":"             +-- manager.py      \u003c-- renamed from mysql/manager_base.py"},{"line_number":122,"context_line":"             +-- service.py      \u003c-- renamed from mysql/service_base.py"},{"line_number":123,"context_line":"            +-- mysql"},{"line_number":124,"context_line":"             +-- __init__.py"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_6c9aa816","line":121,"updated":"2015-10-13 17:08:51.000000000","message":"There was a whole long discussion about the best way to organize the file hierarchy during the original mysql refactor, and this seems to change that.\n\nIn fact, I preferred something closer to what you are proposing here the last time around, but went with the consensus. I\u0027m not seeing any case presented for why it should be changed (and why it\u0027s worth reopening that debate). \n\nIf we have some examples where the refactor caused difficulties that this would address, I\u0027m for it, otherwise I think its better to leave things as they are so that we focus our attention on the more important aspects of this bp","commit_id":"c1e63979d7805c88e7dfcd763e667791c5491c34"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"b6ecd6f34e27df33f71b542c8eb1645862841be1","unresolved":false,"context_lines":[{"line_number":118,"context_line":"                 +-- system.py"},{"line_number":119,"context_line":"            +-- mysql_common        \u003c-- new module"},{"line_number":120,"context_line":"             +-- __init__.py"},{"line_number":121,"context_line":"             +-- manager.py      \u003c-- renamed from mysql/manager_base.py"},{"line_number":122,"context_line":"             +-- service.py      \u003c-- renamed from mysql/service_base.py"},{"line_number":123,"context_line":"            +-- mysql"},{"line_number":124,"context_line":"             +-- __init__.py"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_c077fdd2","line":121,"in_reply_to":"5a2ca52d_6c9aa816","updated":"2015-10-13 18:04:18.000000000","message":"IIRC, the original discussion was just about whether the \u0027base\u0027 directory should be maturity-aware, or maturity agnostic.  This doesn\u0027t change that (it\u0027s still under the \u0027maturity-aware\u0027 directory).\nIf we think they should be together, then I\u0027d tend to lean with Petr\u0027s idea of just having a \u0027mysql\u0027 class, and extending whatever needs it.\nI didn\u0027t mean to start another huge debate, it just felt cleaner this way when I was doing the refactoring...","commit_id":"c1e63979d7805c88e7dfcd763e667791c5491c34"},{"author":{"_account_id":14576,"name":"Petr Malik","email":"pmalik@tesora.com","username":"pmalik"},"change_message_id":"f9c7c624294153cca4902ff0b098623cf925d761","unresolved":false,"context_lines":[{"line_number":138,"context_line":""},{"line_number":139,"context_line":"This manner of determining whether prepare has finished will be accomplished by"},{"line_number":140,"context_line":"having the manager write a file at the start and at the successful conclusion"},{"line_number":141,"context_line":"of the prepare operation.  Any errors (exceptions) will be trapped and logged"},{"line_number":142,"context_line":"and the instance set into the FAILED state."},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"A sample of what this will look like in the base manager is as follows,"},{"line_number":145,"context_line":"although additional properties may be defined as they are deemed necessary (and"}],"source_content_type":"text/x-rst","patch_set":1,"id":"7a2fa921_546fa52a","line":142,"range":{"start_line":141,"start_character":27,"end_line":142,"end_character":43},"updated":"2015-10-08 16:52:36.000000000","message":"We should make it clear that in order for this to work properly the derived datastores should (must) not catch (eat) exceptions as long as they are not going to handle them.\n\nVarious datastores currently follow different behaviors with respect to exceptions (some catch and log them, others re-throw). If a datastore eats an exception that is never going to propagate here or to the API and the user never gets notified (even if the action is a \u0027call\u0027).\n\nI think the base rule should be that datastores always throw to the base (maybe after repacking the exception in a more appropriate TroveError if the cause is a datastore [client] specific exception). The logging should be done here - in the base manager methods - as well (see the next point).\n\nAlso, we should probably set some rules regarding logging. We already log lots of stuff in the API and derived datastores then often log the same information with no additional value (e.g. creating a database X) in their manager and service methods. This just bloats the logs with repetitive messages.\nAgain, I believe, that majority of logging should be done here (the base methods that would do the logging would be just calling \u0027protected\u0027 hooks overriden in derived classes) - derived classes would then just log their specific steps (mostly as DEBUG messages) - i.e. \u0027...resetting MySQL root password after restore...\u0027","commit_id":"c1e63979d7805c88e7dfcd763e667791c5491c34"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"79f6625883c7c7681c827fdbf3c96002bbda666b","unresolved":false,"context_lines":[{"line_number":138,"context_line":""},{"line_number":139,"context_line":"This manner of determining whether prepare has finished will be accomplished by"},{"line_number":140,"context_line":"having the manager write a file at the start and at the successful conclusion"},{"line_number":141,"context_line":"of the prepare operation.  Any errors (exceptions) will be trapped and logged"},{"line_number":142,"context_line":"and the instance set into the FAILED state."},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"A sample of what this will look like in the base manager is as follows,"},{"line_number":145,"context_line":"although additional properties may be defined as they are deemed necessary (and"}],"source_content_type":"text/x-rst","patch_set":1,"id":"7a2fa921_6a0f0a37","line":142,"range":{"start_line":141,"start_character":27,"end_line":142,"end_character":43},"in_reply_to":"7a2fa921_546fa52a","updated":"2015-10-13 17:52:58.000000000","message":"Sounds like basic programming 101 (and by \u0027basic\u0027 I don\u0027t mean the crappy programming language ;) ).  We can make all the policies we want, but I\u0027ll bet that this is stuff we\u0027ll end up having to catch in reviews if needed.","commit_id":"c1e63979d7805c88e7dfcd763e667791c5491c34"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"79f6625883c7c7681c827fdbf3c96002bbda666b","unresolved":false,"context_lines":[{"line_number":232,"context_line":"            self.post_prepare(self, context, packages, databases, memory_mb,"},{"line_number":233,"context_line":"                              users, device_path, mount_point, backup_info,"},{"line_number":234,"context_line":"                              config_contents, root_password, overrides,"},{"line_number":235,"context_line":"                              cluster_config, snapshot):"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"        @abc.abstractmethod"},{"line_number":238,"context_line":"        def do_prepare(self, context, packages, databases, memory_mb, users,"}],"source_content_type":"text/x-rst","patch_set":1,"id":"7a2fa921_fd044615","line":235,"range":{"start_line":235,"start_character":55,"end_line":235,"end_character":56},"updated":"2015-10-13 17:52:58.000000000","message":"Extraneous colon","commit_id":"c1e63979d7805c88e7dfcd763e667791c5491c34"},{"author":{"_account_id":14576,"name":"Petr Malik","email":"pmalik@tesora.com","username":"pmalik"},"change_message_id":"f9c7c624294153cca4902ff0b098623cf925d761","unresolved":false,"context_lines":[{"line_number":252,"context_line":"            \"\"\"This is called after prepare has completed successfully."},{"line_number":253,"context_line":"            Processing done here should be limited to things that will not"},{"line_number":254,"context_line":"            affect the the status of the datastore (for example, creating"},{"line_number":255,"context_line":"            databases and users).  No exceptions are caught and the end-user"},{"line_number":256,"context_line":"            will not be informed of any errors that occur."},{"line_number":257,"context_line":"            \"\"\""},{"line_number":258,"context_line":"            LOG.debug(\u0027No post_prepare work has been defined.\u0027))"},{"line_number":259,"context_line":"            pass"}],"source_content_type":"text/x-rst","patch_set":1,"id":"7a2fa921_745d0146","line":256,"range":{"start_line":255,"start_character":35,"end_line":256,"end_character":58},"updated":"2015-10-08 16:52:36.000000000","message":"Well we should at least log them as in the \u0027do_prepare\u0027 call. It should however be clear that any exception at this stage does not change the datastore status - e.g. creating an instance initialized with databases where the \u0027database-create\u0027 step fails shows up as a valid ACTIVE instance but without the requested databases... with no indication that something went wrong (apart from the messages in the guest-agent log).","commit_id":"c1e63979d7805c88e7dfcd763e667791c5491c34"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"79f6625883c7c7681c827fdbf3c96002bbda666b","unresolved":false,"context_lines":[{"line_number":252,"context_line":"            \"\"\"This is called after prepare has completed successfully."},{"line_number":253,"context_line":"            Processing done here should be limited to things that will not"},{"line_number":254,"context_line":"            affect the the status of the datastore (for example, creating"},{"line_number":255,"context_line":"            databases and users).  No exceptions are caught and the end-user"},{"line_number":256,"context_line":"            will not be informed of any errors that occur."},{"line_number":257,"context_line":"            \"\"\""},{"line_number":258,"context_line":"            LOG.debug(\u0027No post_prepare work has been defined.\u0027))"},{"line_number":259,"context_line":"            pass"}],"source_content_type":"text/x-rst","patch_set":1,"id":"7a2fa921_6a286a9a","line":256,"range":{"start_line":255,"start_character":35,"end_line":256,"end_character":58},"in_reply_to":"7a2fa921_745d0146","updated":"2015-10-13 17:52:58.000000000","message":"Sure, we can log them, but the end user will still never see it (at least not until the logging feature is accepted). ;)","commit_id":"c1e63979d7805c88e7dfcd763e667791c5491c34"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"79f6625883c7c7681c827fdbf3c96002bbda666b","unresolved":false,"context_lines":[{"line_number":255,"context_line":"            databases and users).  No exceptions are caught and the end-user"},{"line_number":256,"context_line":"            will not be informed of any errors that occur."},{"line_number":257,"context_line":"            \"\"\""},{"line_number":258,"context_line":"            LOG.debug(\u0027No post_prepare work has been defined.\u0027))"},{"line_number":259,"context_line":"            pass"},{"line_number":260,"context_line":""},{"line_number":261,"context_line":"        #########################"}],"source_content_type":"text/x-rst","patch_set":1,"id":"7a2fa921_2a059212","line":258,"range":{"start_line":258,"start_character":63,"end_line":258,"end_character":64},"updated":"2015-10-13 17:52:58.000000000","message":"Extraneous parenthesis here ...","commit_id":"c1e63979d7805c88e7dfcd763e667791c5491c34"},{"author":{"_account_id":14576,"name":"Petr Malik","email":"pmalik@tesora.com","username":"pmalik"},"change_message_id":"f9c7c624294153cca4902ff0b098623cf925d761","unresolved":false,"context_lines":[{"line_number":322,"context_line":"            if error_occurred:"},{"line_number":323,"context_line":"                final_status \u003d instance.ServiceStatuses.FAILED"},{"line_number":324,"context_line":"            elif post_processing:"},{"line_number":325,"context_line":"                final_status \u003d instance.ServiceStatuses.INSTANCE_READY"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"            if final_status:"},{"line_number":328,"context_line":"                LOG.info(_(\"Set final status to %s.\") % final_status)"}],"source_content_type":"text/x-rst","patch_set":1,"id":"7a2fa921_3401c906","line":325,"range":{"start_line":325,"start_character":56,"end_line":325,"end_character":70},"updated":"2015-10-08 16:52:36.000000000","message":"I totally agree with this rename. The original name was very confusing. I just thing we should note it somewhere here (Guest Agent/Internal API section?)...","commit_id":"c1e63979d7805c88e7dfcd763e667791c5491c34"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"79f6625883c7c7681c827fdbf3c96002bbda666b","unresolved":false,"context_lines":[{"line_number":322,"context_line":"            if error_occurred:"},{"line_number":323,"context_line":"                final_status \u003d instance.ServiceStatuses.FAILED"},{"line_number":324,"context_line":"            elif post_processing:"},{"line_number":325,"context_line":"                final_status \u003d instance.ServiceStatuses.INSTANCE_READY"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"            if final_status:"},{"line_number":328,"context_line":"                LOG.info(_(\"Set final status to %s.\") % final_status)"}],"source_content_type":"text/x-rst","patch_set":1,"id":"7a2fa921_25f0b190","line":325,"range":{"start_line":325,"start_character":56,"end_line":325,"end_character":70},"in_reply_to":"7a2fa921_3401c906","updated":"2015-10-13 17:52:58.000000000","message":"Right - I\u0027ll mention this in the Internal API section.  Since this only affects things being \u0027built\u0027 there shouldn\u0027t be any backwards compatibility issues.","commit_id":"c1e63979d7805c88e7dfcd763e667791c5491c34"},{"author":{"_account_id":10440,"name":"Alex Tomic","email":"atomic777@gmail.com","username":"atomic77"},"change_message_id":"b820def4ba54ee89b6acf292591c65eea4f0e349","unresolved":false,"context_lines":[{"line_number":471,"context_line":"future work would include testing the base class thoroughly and then removing"},{"line_number":472,"context_line":"all the corresponding tests from the derived classes."},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"The int-tests should continue to run as always, and will be used to determine"},{"line_number":475,"context_line":"that no fundamental changes have been made to the implementation, with the"},{"line_number":476,"context_line":"exception of the fix bugs (and they should just lead to greater stability in"},{"line_number":477,"context_line":"the test infrastructure)."},{"line_number":478,"context_line":""},{"line_number":479,"context_line":""},{"line_number":480,"context_line":"Documentation Impact"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_6252b115","line":477,"range":{"start_line":474,"start_character":0,"end_line":477,"end_character":25},"updated":"2015-10-13 17:08:51.000000000","message":"As we move more logic into base/common classes, it\u0027s getting more important that we have cross-datastore testing. Perhaps part of the scope of this blueprint, or a follow-up, should be to ensure that we have some generic int-tests running as part of the gate, at least in non-voting form to start with.","commit_id":"c1e63979d7805c88e7dfcd763e667791c5491c34"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"b6ecd6f34e27df33f71b542c8eb1645862841be1","unresolved":false,"context_lines":[{"line_number":471,"context_line":"future work would include testing the base class thoroughly and then removing"},{"line_number":472,"context_line":"all the corresponding tests from the derived classes."},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"The int-tests should continue to run as always, and will be used to determine"},{"line_number":475,"context_line":"that no fundamental changes have been made to the implementation, with the"},{"line_number":476,"context_line":"exception of the fix bugs (and they should just lead to greater stability in"},{"line_number":477,"context_line":"the test infrastructure)."},{"line_number":478,"context_line":""},{"line_number":479,"context_line":""},{"line_number":480,"context_line":"Documentation Impact"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_8006e51e","line":477,"range":{"start_line":474,"start_character":0,"end_line":477,"end_character":25},"in_reply_to":"5a2ca52d_6252b115","updated":"2015-10-13 18:04:18.000000000","message":"Well, that\u0027s a whole bigger scope.  We\u0027ve talked about having 3rd-party CI\u0027s (and it would be a huge benefit) however if we make this refactory dependent on having 3rd-party CI\u0027s, I\u0027m thinking that it\u0027ll never get done...","commit_id":"c1e63979d7805c88e7dfcd763e667791c5491c34"}]}