)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"fdcd96d5b3653c9ea6a48fdd4f029bc309e912f4","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Fix PostgreSQL volume definitions"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Add support for resize-volume."},{"line_number":10,"context_line":"Raise exceptions for unsupported features."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Ifc245dbf27af0269a2b71995bc2c6e2733420a81"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"1a930d6b_89c3faeb","line":10,"updated":"2015-01-26 20:44:24.000000000","message":"I agree with Kevin on creating a new patchset, but I think it should be on this part.  The two changes are unrelated (fixing volumes and raising exceptions for unsupported features).  Just in case they may need to be backported separately, it would be better if you spin this off as a separate bug/patchset.","commit_id":"b8767606e604ff4b79d11d7112cc06ae14fb4f7e"},{"author":{"_account_id":14176,"name":"Nirav Shah","email":"nshah@tesora.com","username":"nshah"},"change_message_id":"6ead5f77a8698c783d564bf7ea366890aa771b2c","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Fix PostgreSQL volume definitions"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Add support for resize-volume."},{"line_number":10,"context_line":"Raise exceptions for unsupported features."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Ifc245dbf27af0269a2b71995bc2c6e2733420a81"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"1a930d6b_790a1727","line":10,"in_reply_to":"1a930d6b_89c3faeb","updated":"2015-01-26 22:54:09.000000000","message":"Agreed.","commit_id":"b8767606e604ff4b79d11d7112cc06ae14fb4f7e"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"fdcd96d5b3653c9ea6a48fdd4f029bc309e912f4","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Add support for resize-volume."},{"line_number":10,"context_line":"Raise exceptions for unsupported features."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Ifc245dbf27af0269a2b71995bc2c6e2733420a81"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"1a930d6b_a93996f3","line":11,"updated":"2015-01-26 20:44:24.000000000","message":"You lost the bug id in this patchset.  Also, could you include more information from the bug here? Without looking at the bug it\u0027s impossible to understand some of the changes (i.e. why the do_not_start_on_reboot\u003dFalse flag was necessary).  See https://wiki.openstack.org/wiki/GitCommitMessages#Information_in_commit_messages for some good example.","commit_id":"b8767606e604ff4b79d11d7112cc06ae14fb4f7e"},{"author":{"_account_id":14176,"name":"Nirav Shah","email":"nshah@tesora.com","username":"nshah"},"change_message_id":"6ead5f77a8698c783d564bf7ea366890aa771b2c","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Add support for resize-volume."},{"line_number":10,"context_line":"Raise exceptions for unsupported features."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Ifc245dbf27af0269a2b71995bc2c6e2733420a81"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"1a930d6b_5c16294d","line":11,"in_reply_to":"1a930d6b_a93996f3","updated":"2015-01-26 22:54:09.000000000","message":"I\u0027ll add the bug number again, and have more details in the commit message.","commit_id":"b8767606e604ff4b79d11d7112cc06ae14fb4f7e"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"97963de585ec9aeb21e8ed69f92432367f2232b8","unresolved":false,"context_lines":[{"line_number":18,"context_line":"in postgresql)."},{"line_number":19,"context_line":"The easiest way to verify this bug (and this patch) is to follow"},{"line_number":20,"context_line":"these steps:"},{"line_number":21,"context_line":"- trove create ps1 100 --datastore postgresql --datastore_version"},{"line_number":22,"context_line":"  9.3 --size 1 --databases db1 --users user1:password1"},{"line_number":23,"context_line":"- trove list (note the ID, and verify instance is active)"},{"line_number":24,"context_line":"- trove resize-volume \u003cid\u003e 2"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"da86d52c_235b1c55","line":21,"updated":"2015-02-18 17:42:32.000000000","message":"100 must be a custom flavor, I\u0027ll test with 7","commit_id":"69773141dd194a9c06685b86e2e88fd78d0bd314"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"97963de585ec9aeb21e8ed69f92432367f2232b8","unresolved":false,"context_lines":[{"line_number":19,"context_line":"The easiest way to verify this bug (and this patch) is to follow"},{"line_number":20,"context_line":"these steps:"},{"line_number":21,"context_line":"- trove create ps1 100 --datastore postgresql --datastore_version"},{"line_number":22,"context_line":"  9.3 --size 1 --databases db1 --users user1:password1"},{"line_number":23,"context_line":"- trove list (note the ID, and verify instance is active)"},{"line_number":24,"context_line":"- trove resize-volume \u003cid\u003e 2"},{"line_number":25,"context_line":"- trove list (verify that size is 2)"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"da86d52c_c337a0fc","line":22,"updated":"2015-02-18 17:42:32.000000000","message":"Trove supports postgresql 9.1 at present ...","commit_id":"69773141dd194a9c06685b86e2e88fd78d0bd314"}],"trove/guestagent/datastore/postgresql/manager.py":[{"author":{"_account_id":7850,"name":"Kevin Conway","email":"kevinjacobconway@gmail.com","username":"kevinconway"},"change_message_id":"52094fdb9f67cca144a12f1c9d7d10eba9157b0f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"1a930d6b_67c9cd58","updated":"2015-01-24 00:05:05.000000000","message":"For this much functionality I ask that you move related features to a mixin like the others features of the PostgreSQL agent.\nThe override functions likely should go in the config mixin which already exists in the \u0027services\u0027 package. You can create a volume and ha/clustering mixin for the other functionality.\nAlso, please follow the convention of the other PostgreSQL service mixins by documenting the function parameters and output. Having the guest agent contract documented makes it _much_ easier to debug and identify when things have changed. See https://github.com/openstack/trove/blob/master/trove/guestagent/datastore/postgresql/service/users.py for some examples.","commit_id":"c430313d10c8a8c5a942d1c6b3caf80c3ba70096"},{"author":{"_account_id":14576,"name":"Petr Malik","email":"pmalik@tesora.com","username":"pmalik"},"change_message_id":"8b27663ee71b61cc2099e07786ad516ff8bf79c8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"1a930d6b_e0381a3f","in_reply_to":"1a930d6b_0acabc1a","updated":"2015-01-26 17:07:04.000000000","message":"I agree that we should keep the datastore architectures consistent.\nWe have also already tried to implement unsupported methods using mixins and it got vastly rejected by the community.","commit_id":"c430313d10c8a8c5a942d1c6b3caf80c3ba70096"},{"author":{"_account_id":14176,"name":"Nirav Shah","email":"nshah@tesora.com","username":"nshah"},"change_message_id":"3b06d55b313696ced5be1bae998b4964f7482b45","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"1a930d6b_0acabc1a","in_reply_to":"1a930d6b_67c9cd58","updated":"2015-01-26 16:18:52.000000000","message":"Thanks for your comment. The reason why I decided to implement this functionality here, was to stay consistent with other datastores\u0027 implementation. I\u0027m actually not sure why we use mixin classes for postgresql, when all other datastores (cassandra, couchbase, mongodb, mysql, redis) follow the conventional route. I\u0027m just afraid that we\u0027ll run into hard-to-debug MRO issues eventually, if we end up having same method names for different mixin classes. \n\nIf there is a strong enough consensus from the community to change the current implementation, I can definitely do it the mixin way. But at this point, it just seems like a semantics issue. The current implementation has been tested for \u0027resize-volume\u0027 functionality, and it works.","commit_id":"c430313d10c8a8c5a942d1c6b3caf80c3ba70096"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"a2bf362e3f5600bc9d4600ba7710778d4bca5bfa","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"fa81d914_4e00caaa","in_reply_to":"1a930d6b_e0381a3f","updated":"2015-01-28 08:59:24.000000000","message":"I don\u0027t think it\u0027s just a semantics issue. Having a mixin class for something like the base unsupported methods seems like a fairly good idea since it would mean that you wouldn\u0027t have to repeat yourself over for each datastore that didn\u0027t support these base methods. This would in turn shrink the LOC that you\u0027d have to maintain in the project. I\u0027m curious to see the review where this was rejected by folks. Do you have a link? Thanks.","commit_id":"c430313d10c8a8c5a942d1c6b3caf80c3ba70096"},{"author":{"_account_id":14576,"name":"Petr Malik","email":"pmalik@tesora.com","username":"pmalik"},"change_message_id":"016dde8ab2e9206f106e589e0311778306e9bed8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"fa81d914_ad8cbe36","in_reply_to":"fa81d914_171ed470","updated":"2015-01-29 01:15:48.000000000","message":"I agree with Nirav. I would suggest we keep the current patch set consistent with other datastores and file a new blueprint to re-architect the existing managers as suggested (or at least bring them all in line).","commit_id":"c430313d10c8a8c5a942d1c6b3caf80c3ba70096"},{"author":{"_account_id":7850,"name":"Kevin Conway","email":"kevinjacobconway@gmail.com","username":"kevinconway"},"change_message_id":"625e130bc2cf5b5596352f8a9f6dbcc2caf8905d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"fa81d914_48f3309e","in_reply_to":"fa81d914_171ed470","updated":"2015-01-29 01:45:13.000000000","message":"I\u0027m not suggesting you refactor every agent to use mixins. I\u0027m asking that you follow the clearly defined organization of this particular guest agent.\n\nI understand the FUD you are expressing with regards to the Python MRO, but that only becomes an issue in a _real_ mixin architecture. Our use of mixin\u0027s for this guest agent are primarily for code organization and quite trivial. It keeps related functionality packaged together and somewhat separate from all the other logic. I\u0027ve found it easier to debug when there isn\u0027t a sea of clutter made up of unrelated functionality in a module. We aren\u0027t creating super generic mixin\u0027s which can be included in any Python class. We\u0027re just categorizing and separating concerns.\n\nPS For those curious about the Python MRO check out https://www.python.org/download/releases/2.3/mro/ for a detailed write-up.","commit_id":"c430313d10c8a8c5a942d1c6b3caf80c3ba70096"},{"author":{"_account_id":14176,"name":"Nirav Shah","email":"nshah@tesora.com","username":"nshah"},"change_message_id":"64bf1a394fe704a0b76593fb48b15fc25b2ab0e2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"fa81d914_171ed470","in_reply_to":"fa81d914_4e00caaa","updated":"2015-01-28 16:46:36.000000000","message":"There are two different issues here:\n1. Usage of mixin class in postgresql is inconsistent with other datastores\u0027 implementation that do not have mixin classes. In postgresql, you will notice that instead of having service.py, postgresql has service directory and a bunch of modules within. It also has Manager class that has multiple inheritance from all the other service classes. This can be error-prone, since people now have to be careful to not have similar named definitions (technically, this would be allowed, but it would be nightmarish to debug any potential problem). \n2. It is definitely a better idea to have a base class that has all the definitions defined. We briefly talked about this in IRC. These base classes\u0027 definitions can just throw DatastoreOperationNotSupported exceptions. Then, each datastore manager can inherit from this base class, and just implement all the supported definitions. But as you can see, this is different from mixins that require multiple inheritance. This implementation could be significant work, and might require a blueprint. There was an attempt last year to get this base class implementation going (here is the patch: https://review.openstack.org/#/c/78054/), but I think it was mostly rejected, because it was beyond the scope of the original bug. So I\u0027ll work on a blueprint later, and we can discuss that when it\u0027s ready.\n\nIn any case, the above (especially issue# 2) is beyond the scope of this patch: get postgresql resize-volume working.\n\nPlease let me know your thoughts/concerns.","commit_id":"c430313d10c8a8c5a942d1c6b3caf80c3ba70096"},{"author":{"_account_id":7850,"name":"Kevin Conway","email":"kevinjacobconway@gmail.com","username":"kevinconway"},"change_message_id":"52094fdb9f67cca144a12f1c9d7d10eba9157b0f","unresolved":false,"context_lines":[{"line_number":102,"context_line":"    def mount_volume(self, context, device_path\u003dNone, mount_point\u003dNone):"},{"line_number":103,"context_line":"        device \u003d volume.VolumeDevice(device_path)"},{"line_number":104,"context_line":"        device.mount(mount_point, write_to_fstab\u003dFalse)"},{"line_number":105,"context_line":"        LOG.debug(\"Mounted the device %s at the mount point %s.\" %"},{"line_number":106,"context_line":"                  (device_path, mount_point))"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    def unmount_volume(self, context, device_path\u003dNone, mount_point\u003dNone):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a930d6b_27eb45b1","line":105,"updated":"2015-01-24 00:05:05.000000000","message":"Either use \u0027.format()\u0027 for string interpolation like the rest of the PostgreSQL code or pass the values in as additional parameters to the logger and let it handle everything. Should, in this particular example, the device_path or mount_point be something other than strings the \u0027%\u0027 format will fail and an exception will be thrown. \u0027.format()\u0027 does not have this problem and will log the \u0027repr()\u0027 of some foreign object instead.\nFor reference: https://docs.python.org/2/library/string.html#format-string-syntax\n\nI don\u0027t expect problems like that to come up often, but it is painful when the debug line you added for debugging ends up being the bug you are debugging.","commit_id":"c430313d10c8a8c5a942d1c6b3caf80c3ba70096"},{"author":{"_account_id":14176,"name":"Nirav Shah","email":"nshah@tesora.com","username":"nshah"},"change_message_id":"3b06d55b313696ced5be1bae998b4964f7482b45","unresolved":false,"context_lines":[{"line_number":102,"context_line":"    def mount_volume(self, context, device_path\u003dNone, mount_point\u003dNone):"},{"line_number":103,"context_line":"        device \u003d volume.VolumeDevice(device_path)"},{"line_number":104,"context_line":"        device.mount(mount_point, write_to_fstab\u003dFalse)"},{"line_number":105,"context_line":"        LOG.debug(\"Mounted the device %s at the mount point %s.\" %"},{"line_number":106,"context_line":"                  (device_path, mount_point))"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    def unmount_volume(self, context, device_path\u003dNone, mount_point\u003dNone):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a930d6b_afae1ae4","line":105,"in_reply_to":"1a930d6b_27eb45b1","updated":"2015-01-26 16:18:52.000000000","message":"Good catch. I\u0027ll make this change, and send along another patch.","commit_id":"c430313d10c8a8c5a942d1c6b3caf80c3ba70096"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"fdcd96d5b3653c9ea6a48fdd4f029bc309e912f4","unresolved":false,"context_lines":[{"line_number":120,"context_line":"            \"Resized the filesystem at {mount}.\".format("},{"line_number":121,"context_line":"                mount\u003dmount_point))"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"    def update_overrides(self, context, overrides, remove\u003dFalse):"},{"line_number":124,"context_line":"        LOG.debug(\"Updating overrides.\")"},{"line_number":125,"context_line":"        raise exception.DatastoreOperationNotSupported("},{"line_number":126,"context_line":"            operation\u003d\u0027update_overrides\u0027, datastore\u003dMANAGER)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a930d6b_445019d4","line":123,"updated":"2015-01-26 20:44:24.000000000","message":"I think these 6 methods should be in a new bug/commit","commit_id":"b8767606e604ff4b79d11d7112cc06ae14fb4f7e"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"fc409c0aa7be61b0b61263f53087a5e53a7743f0","unresolved":false,"context_lines":[{"line_number":120,"context_line":"            \"Resized the filesystem at {mount}.\".format("},{"line_number":121,"context_line":"                mount\u003dmount_point))"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"    def update_overrides(self, context, overrides, remove\u003dFalse):"},{"line_number":124,"context_line":"        LOG.debug(\"Updating overrides.\")"},{"line_number":125,"context_line":"        raise exception.DatastoreOperationNotSupported("},{"line_number":126,"context_line":"            operation\u003d\u0027update_overrides\u0027, datastore\u003dMANAGER)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a930d6b_d9294b8b","line":123,"in_reply_to":"1a930d6b_445019d4","updated":"2015-01-26 22:29:51.000000000","message":"+1","commit_id":"b8767606e604ff4b79d11d7112cc06ae14fb4f7e"},{"author":{"_account_id":14176,"name":"Nirav Shah","email":"nshah@tesora.com","username":"nshah"},"change_message_id":"6ead5f77a8698c783d564bf7ea366890aa771b2c","unresolved":false,"context_lines":[{"line_number":120,"context_line":"            \"Resized the filesystem at {mount}.\".format("},{"line_number":121,"context_line":"                mount\u003dmount_point))"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"    def update_overrides(self, context, overrides, remove\u003dFalse):"},{"line_number":124,"context_line":"        LOG.debug(\"Updating overrides.\")"},{"line_number":125,"context_line":"        raise exception.DatastoreOperationNotSupported("},{"line_number":126,"context_line":"            operation\u003d\u0027update_overrides\u0027, datastore\u003dMANAGER)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a930d6b_d91b0b7f","line":123,"in_reply_to":"1a930d6b_d9294b8b","updated":"2015-01-26 22:54:09.000000000","message":"I\u0027ve filed bug# 1414813. I\u0027ll remove these definitions from here, and will add them for this new bug.","commit_id":"b8767606e604ff4b79d11d7112cc06ae14fb4f7e"},{"author":{"_account_id":7850,"name":"Kevin Conway","email":"kevinjacobconway@gmail.com","username":"kevinconway"},"change_message_id":"cbb6971575c46a4416a872517df9f976d227c2cd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"fa81d914_489410e2","updated":"2015-01-29 01:44:20.000000000","message":"Asking again, please add docstrings to any guest agent methods you add containing the input/output specifications.\n\nWhen someone else has to drop in to modify the method it helps to know what exactly they have to work with. It also helps a ton when someone changes the guest agent communication contract without warning because you would then have some idea of what _should_ be there.","commit_id":"9bfbcaf159f000759646ef93966a25533a7b0ba2"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"0e621a1f4474852c89c712c0b5970b181fde7f9d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"fa81d914_f1f00c69","in_reply_to":"fa81d914_489410e2","updated":"2015-01-30 12:13:18.000000000","message":"Kevin, agreed, it does make sense to do this.","commit_id":"9bfbcaf159f000759646ef93966a25533a7b0ba2"},{"author":{"_account_id":7806,"name":"Sushil Kumar","email":"skm.net@gmail.com","username":"sushilkm"},"change_message_id":"d8e177623c2de22eae8397129d001e503252483b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"da86d52c_05784a2a","in_reply_to":"fa81d914_f1f00c69","updated":"2015-02-02 18:58:20.000000000","message":"+1 for docstrings","commit_id":"9bfbcaf159f000759646ef93966a25533a7b0ba2"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"19fbcf10d825c913202a6fd8d7b716bdfa135c18","unresolved":false,"context_lines":[{"line_number":103,"context_line":"        \"\"\"Mount the volume as specified by device_path to mount_point.\"\"\""},{"line_number":104,"context_line":"        device \u003d volume.VolumeDevice(device_path)"},{"line_number":105,"context_line":"        device.mount(mount_point, write_to_fstab\u003dFalse)"},{"line_number":106,"context_line":"        LOG.debug("},{"line_number":107,"context_line":"            \"Mounted device {device} at mount point {mount}.\".format("},{"line_number":108,"context_line":"                device\u003ddevice_path, mount\u003dmount_point))"},{"line_number":109,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_348dae7c","line":106,"updated":"2015-02-20 12:28:05.000000000","message":"In the rare and infrequent times when this method is called, if LOG.debug is not enabled, you perform one extra string rendering.\n\nNo need to respin this patch but I think the direction we\u0027re heading in is to pass LOG.debug a format string and parameters as a dictionary and allow it to do the rendering if debug is enabled.","commit_id":"69773141dd194a9c06685b86e2e88fd78d0bd314"},{"author":{"_account_id":14176,"name":"Nirav Shah","email":"nshah@tesora.com","username":"nshah"},"change_message_id":"19ea6c06e132da70dd1436433dd66247b07e633f","unresolved":false,"context_lines":[{"line_number":103,"context_line":"        \"\"\"Mount the volume as specified by device_path to mount_point.\"\"\""},{"line_number":104,"context_line":"        device \u003d volume.VolumeDevice(device_path)"},{"line_number":105,"context_line":"        device.mount(mount_point, write_to_fstab\u003dFalse)"},{"line_number":106,"context_line":"        LOG.debug("},{"line_number":107,"context_line":"            \"Mounted device {device} at mount point {mount}.\".format("},{"line_number":108,"context_line":"                device\u003ddevice_path, mount\u003dmount_point))"},{"line_number":109,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_e0cc94bd","line":106,"in_reply_to":"ba7be1f8_348dae7c","updated":"2015-03-03 17:57:26.000000000","message":"Good to know! Thanks.","commit_id":"69773141dd194a9c06685b86e2e88fd78d0bd314"}],"trove/guestagent/datastore/postgresql/service/process.py":[{"author":{"_account_id":7850,"name":"Kevin Conway","email":"kevinjacobconway@gmail.com","username":"kevinconway"},"change_message_id":"52094fdb9f67cca144a12f1c9d7d10eba9157b0f","unresolved":false,"context_lines":[{"line_number":43,"context_line":"            timeout\u003d30"},{"line_number":44,"context_line":"        )"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def _disable_pgsql_on_boot(self):"},{"line_number":47,"context_line":"        try:"},{"line_number":48,"context_line":"            pgsql_service \u003d operating_system.service_discovery("},{"line_number":49,"context_line":"                PGSQL_SERVICE_CANDIDATES)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a930d6b_67d0edf6","line":46,"updated":"2015-01-24 00:05:05.000000000","message":"This change does not appear to be in the scope of the commit message. You should consider spinning this off into another patch which adds both disable and enable on boot. It doesn\u0027t make sense to only have one.","commit_id":"c430313d10c8a8c5a942d1c6b3caf80c3ba70096"},{"author":{"_account_id":14176,"name":"Nirav Shah","email":"nshah@tesora.com","username":"nshah"},"change_message_id":"3b06d55b313696ced5be1bae998b4964f7482b45","unresolved":false,"context_lines":[{"line_number":43,"context_line":"            timeout\u003d30"},{"line_number":44,"context_line":"        )"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def _disable_pgsql_on_boot(self):"},{"line_number":47,"context_line":"        try:"},{"line_number":48,"context_line":"            pgsql_service \u003d operating_system.service_discovery("},{"line_number":49,"context_line":"                PGSQL_SERVICE_CANDIDATES)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a930d6b_4ad044c9","line":46,"in_reply_to":"1a930d6b_67d0edf6","updated":"2015-01-26 16:18:52.000000000","message":"Good point about adding enable support. I\u0027ll add that.","commit_id":"c430313d10c8a8c5a942d1c6b3caf80c3ba70096"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"aa24e7e8170f51c42b60879f092397f1dd30675e","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        try:"},{"line_number":48,"context_line":"            pgsql_service \u003d operating_system.service_discovery("},{"line_number":49,"context_line":"                PGSQL_SERVICE_CANDIDATES)"},{"line_number":50,"context_line":"            utils.execute_with_timeout(pgsql_service[\u0027cmd_disable\u0027],"},{"line_number":51,"context_line":"                                       shell\u003dTrue)"},{"line_number":52,"context_line":"        except KeyError:"},{"line_number":53,"context_line":"            LOG.exception(_(\"Error disabling PostgreSQL start on boot.\"))"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a930d6b_8947a8bb","line":50,"updated":"2015-01-24 13:16:28.000000000","message":"not sure I see why you need shell \u003d True here.","commit_id":"c430313d10c8a8c5a942d1c6b3caf80c3ba70096"},{"author":{"_account_id":14176,"name":"Nirav Shah","email":"nshah@tesora.com","username":"nshah"},"change_message_id":"3b06d55b313696ced5be1bae998b4964f7482b45","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        try:"},{"line_number":48,"context_line":"            pgsql_service \u003d operating_system.service_discovery("},{"line_number":49,"context_line":"                PGSQL_SERVICE_CANDIDATES)"},{"line_number":50,"context_line":"            utils.execute_with_timeout(pgsql_service[\u0027cmd_disable\u0027],"},{"line_number":51,"context_line":"                                       shell\u003dTrue)"},{"line_number":52,"context_line":"        except KeyError:"},{"line_number":53,"context_line":"            LOG.exception(_(\"Error disabling PostgreSQL start on boot.\"))"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a930d6b_7a9a176f","line":50,"in_reply_to":"1a930d6b_8947a8bb","updated":"2015-01-26 16:18:52.000000000","message":"Good point. Let me retest without \u0027shell\u0027, and see if it works...","commit_id":"c430313d10c8a8c5a942d1c6b3caf80c3ba70096"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"fdcd96d5b3653c9ea6a48fdd4f029bc309e912f4","unresolved":false,"context_lines":[{"line_number":43,"context_line":"            timeout\u003d30"},{"line_number":44,"context_line":"        )"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def _enable_pgsql_on_boot(self):"},{"line_number":47,"context_line":"        try:"},{"line_number":48,"context_line":"            pgsql_service \u003d operating_system.service_discovery("},{"line_number":49,"context_line":"                PGSQL_SERVICE_CANDIDATES)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a930d6b_c47169a9","line":46,"updated":"2015-01-26 20:44:24.000000000","message":"This method isn\u0027t called from anywhere (maybe an oversight?).  I understand the idea of \u0027completeness,\u0027 however adding code that isn\u0027t used probably isn\u0027t a good idea.","commit_id":"b8767606e604ff4b79d11d7112cc06ae14fb4f7e"},{"author":{"_account_id":14176,"name":"Nirav Shah","email":"nshah@tesora.com","username":"nshah"},"change_message_id":"6ead5f77a8698c783d564bf7ea366890aa771b2c","unresolved":false,"context_lines":[{"line_number":43,"context_line":"            timeout\u003d30"},{"line_number":44,"context_line":"        )"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def _enable_pgsql_on_boot(self):"},{"line_number":47,"context_line":"        try:"},{"line_number":48,"context_line":"            pgsql_service \u003d operating_system.service_discovery("},{"line_number":49,"context_line":"                PGSQL_SERVICE_CANDIDATES)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a930d6b_598c7bda","line":46,"in_reply_to":"1a930d6b_395a4f2a","updated":"2015-01-26 22:54:09.000000000","message":"Sure. Will add this call in start_db. I see that other datastores do this.","commit_id":"b8767606e604ff4b79d11d7112cc06ae14fb4f7e"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"fc409c0aa7be61b0b61263f53087a5e53a7743f0","unresolved":false,"context_lines":[{"line_number":43,"context_line":"            timeout\u003d30"},{"line_number":44,"context_line":"        )"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def _enable_pgsql_on_boot(self):"},{"line_number":47,"context_line":"        try:"},{"line_number":48,"context_line":"            pgsql_service \u003d operating_system.service_discovery("},{"line_number":49,"context_line":"                PGSQL_SERVICE_CANDIDATES)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a930d6b_395a4f2a","line":46,"in_reply_to":"1a930d6b_c47169a9","updated":"2015-01-26 22:29:51.000000000","message":"this method should be called at start_db, see other implementation.","commit_id":"b8767606e604ff4b79d11d7112cc06ae14fb4f7e"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"fc409c0aa7be61b0b61263f53087a5e53a7743f0","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        try:"},{"line_number":48,"context_line":"            pgsql_service \u003d operating_system.service_discovery("},{"line_number":49,"context_line":"                PGSQL_SERVICE_CANDIDATES)"},{"line_number":50,"context_line":"            utils.execute_with_timeout(pgsql_service[\u0027cmd_enable\u0027])"},{"line_number":51,"context_line":"        except KeyError:"},{"line_number":52,"context_line":"            LOG.exception(_(\"Error enabling PostgreSQL start on boot.\"))"},{"line_number":53,"context_line":"            raise RuntimeError(\"Service is not discovered.\")"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a930d6b_f9a1a7f8","line":50,"updated":"2015-01-26 22:29:51.000000000","message":"shell\u003dTrue","commit_id":"b8767606e604ff4b79d11d7112cc06ae14fb4f7e"},{"author":{"_account_id":14176,"name":"Nirav Shah","email":"nshah@tesora.com","username":"nshah"},"change_message_id":"6ead5f77a8698c783d564bf7ea366890aa771b2c","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        try:"},{"line_number":48,"context_line":"            pgsql_service \u003d operating_system.service_discovery("},{"line_number":49,"context_line":"                PGSQL_SERVICE_CANDIDATES)"},{"line_number":50,"context_line":"            utils.execute_with_timeout(pgsql_service[\u0027cmd_enable\u0027])"},{"line_number":51,"context_line":"        except KeyError:"},{"line_number":52,"context_line":"            LOG.exception(_(\"Error enabling PostgreSQL start on boot.\"))"},{"line_number":53,"context_line":"            raise RuntimeError(\"Service is not discovered.\")"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a930d6b_39b32fea","line":50,"in_reply_to":"1a930d6b_f9a1a7f8","updated":"2015-01-26 22:54:09.000000000","message":"I actually initially had shell\u003dTrue in disable def, but it seems to work without this additional argument. I see that other datastores\u0027 implementation has shell\u003dTrue. What is the reasoning behind that?","commit_id":"b8767606e604ff4b79d11d7112cc06ae14fb4f7e"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"fdcd96d5b3653c9ea6a48fdd4f029bc309e912f4","unresolved":false,"context_lines":[{"line_number":52,"context_line":"            LOG.exception(_(\"Error enabling PostgreSQL start on boot.\"))"},{"line_number":53,"context_line":"            raise RuntimeError(\"Service is not discovered.\")"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def _disable_pgsql_on_boot(self):"},{"line_number":56,"context_line":"        try:"},{"line_number":57,"context_line":"            pgsql_service \u003d operating_system.service_discovery("},{"line_number":58,"context_line":"                PGSQL_SERVICE_CANDIDATES)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a930d6b_84a201fb","line":55,"updated":"2015-01-26 20:44:24.000000000","message":"If you update the commit message, the reason for this addition will become obvious.","commit_id":"b8767606e604ff4b79d11d7112cc06ae14fb4f7e"},{"author":{"_account_id":14176,"name":"Nirav Shah","email":"nshah@tesora.com","username":"nshah"},"change_message_id":"6ead5f77a8698c783d564bf7ea366890aa771b2c","unresolved":false,"context_lines":[{"line_number":52,"context_line":"            LOG.exception(_(\"Error enabling PostgreSQL start on boot.\"))"},{"line_number":53,"context_line":"            raise RuntimeError(\"Service is not discovered.\")"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def _disable_pgsql_on_boot(self):"},{"line_number":56,"context_line":"        try:"},{"line_number":57,"context_line":"            pgsql_service \u003d operating_system.service_discovery("},{"line_number":58,"context_line":"                PGSQL_SERVICE_CANDIDATES)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a930d6b_19d113cd","line":55,"in_reply_to":"1a930d6b_84a201fb","updated":"2015-01-26 22:54:09.000000000","message":"Sure. Will update the commit message.","commit_id":"b8767606e604ff4b79d11d7112cc06ae14fb4f7e"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"0ace0a785d609743303add4fe2ec2429fe9aa90c","unresolved":false,"context_lines":[{"line_number":49,"context_line":"            pgsql_service \u003d operating_system.service_discovery("},{"line_number":50,"context_line":"                PGSQL_SERVICE_CANDIDATES)"},{"line_number":51,"context_line":"            utils.execute_with_timeout(pgsql_service[\u0027cmd_enable\u0027],"},{"line_number":52,"context_line":"                                       shell\u003dTrue)"},{"line_number":53,"context_line":"        except KeyError:"},{"line_number":54,"context_line":"            LOG.exception(_(\"Error enabling PostgreSQL start on boot.\"))"},{"line_number":55,"context_line":"            raise RuntimeError(\"Service is not discovered.\")"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a930d6b_86d66026","line":52,"updated":"2015-01-27 16:08:00.000000000","message":"shell\u003dTrue is passed down to the subprocess.Popen command and basically allows you to use shell constructs such as \u0027\u003e\u003e\u0027 and \u0027||\u0027 and \u0027;\u0027 - without this, parts of the command might fail (while having enough run to make it look liked it worked, at least at first).  Using it can be a security risk if the input is in any way user-defined (can result in \u0027shell injection\u0027) and as such is highly discouraged, however in this case the commands are hard-coded in operating_system.py, so we\u0027re good.","commit_id":"9bfbcaf159f000759646ef93966a25533a7b0ba2"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"f5f7488a5c0711285ee16289b72935b0c860d9d2","unresolved":false,"context_lines":[{"line_number":49,"context_line":"            pgsql_service \u003d operating_system.service_discovery("},{"line_number":50,"context_line":"                PGSQL_SERVICE_CANDIDATES)"},{"line_number":51,"context_line":"            utils.execute_with_timeout(pgsql_service[\u0027cmd_enable\u0027],"},{"line_number":52,"context_line":"                                       shell\u003dTrue)"},{"line_number":53,"context_line":"        except KeyError:"},{"line_number":54,"context_line":"            LOG.exception(_(\"Error enabling PostgreSQL start on boot.\"))"},{"line_number":55,"context_line":"            raise RuntimeError(\"Service is not discovered.\")"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa81d914_0eced25a","line":52,"in_reply_to":"1a930d6b_20daef16","updated":"2015-01-28 08:59:13.000000000","message":"Yes, I understand the scenario where shell\u003dtrue is needed (i.e. interaction with pipes, etc). I don\u0027t think \"cmd_enable\" requires any of these interactions, so I think we should be okay running without \"shell\u003dTrue\". \n\nThe issue you might be running into which is causing this to fail though is improper tokenization of your command since you no loger have a shell doing this tokenization for you. Consider using .split() or shlex.split() to get the right tokenization in this case.","commit_id":"9bfbcaf159f000759646ef93966a25533a7b0ba2"},{"author":{"_account_id":14176,"name":"Nirav Shah","email":"nshah@tesora.com","username":"nshah"},"change_message_id":"739eda5a6485875680a05a88624cbc78a1ff663c","unresolved":false,"context_lines":[{"line_number":49,"context_line":"            pgsql_service \u003d operating_system.service_discovery("},{"line_number":50,"context_line":"                PGSQL_SERVICE_CANDIDATES)"},{"line_number":51,"context_line":"            utils.execute_with_timeout(pgsql_service[\u0027cmd_enable\u0027],"},{"line_number":52,"context_line":"                                       shell\u003dTrue)"},{"line_number":53,"context_line":"        except KeyError:"},{"line_number":54,"context_line":"            LOG.exception(_(\"Error enabling PostgreSQL start on boot.\"))"},{"line_number":55,"context_line":"            raise RuntimeError(\"Service is not discovered.\")"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a930d6b_20daef16","line":52,"in_reply_to":"1a930d6b_71e12c29","updated":"2015-01-27 17:08:42.000000000","message":"Everything mentioned by Peter is correct. \nWithout shell\u003dTrue, start_db (which now has _enable_pgsql_on_boot) started failing.","commit_id":"9bfbcaf159f000759646ef93966a25533a7b0ba2"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"2f8a6b6a22503d87d1fc8b449809502aee87ae9a","unresolved":false,"context_lines":[{"line_number":49,"context_line":"            pgsql_service \u003d operating_system.service_discovery("},{"line_number":50,"context_line":"                PGSQL_SERVICE_CANDIDATES)"},{"line_number":51,"context_line":"            utils.execute_with_timeout(pgsql_service[\u0027cmd_enable\u0027],"},{"line_number":52,"context_line":"                                       shell\u003dTrue)"},{"line_number":53,"context_line":"        except KeyError:"},{"line_number":54,"context_line":"            LOG.exception(_(\"Error enabling PostgreSQL start on boot.\"))"},{"line_number":55,"context_line":"            raise RuntimeError(\"Service is not discovered.\")"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a930d6b_71e12c29","line":52,"in_reply_to":"1a930d6b_86d66026","updated":"2015-01-27 16:24:14.000000000","message":"why do you need shell\u003dTrue here?","commit_id":"9bfbcaf159f000759646ef93966a25533a7b0ba2"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"850ac44ac4c3e61b61fb1c6a611ab42c7f784ccf","unresolved":false,"context_lines":[{"line_number":49,"context_line":"            pgsql_service \u003d operating_system.service_discovery("},{"line_number":50,"context_line":"                PGSQL_SERVICE_CANDIDATES)"},{"line_number":51,"context_line":"            utils.execute_with_timeout(pgsql_service[\u0027cmd_enable\u0027],"},{"line_number":52,"context_line":"                                       shell\u003dTrue)"},{"line_number":53,"context_line":"        except KeyError:"},{"line_number":54,"context_line":"            LOG.exception(_(\"Error enabling PostgreSQL start on boot.\"))"},{"line_number":55,"context_line":"            raise RuntimeError(\"Service is not discovered.\")"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa81d914_e45d1d4a","line":52,"in_reply_to":"fa81d914_0eced25a","updated":"2015-01-28 10:03:43.000000000","message":"Nikhil, the issue isn\u0027t for Ubuntu but for for cases where (for example) one uses upstart where the command would look like:\n\nsudo sh -c \u0027echo manual \u003e\u003e /etc/init/%s.conf\n\nwhich does require shell\u003dTrue\n\nWorking on a fix for this in o.c but that hasn\u0027t landed yet.","commit_id":"9bfbcaf159f000759646ef93966a25533a7b0ba2"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"90bbd1acf583556a234bb1fdfa8e0538d31f0473","unresolved":false,"context_lines":[{"line_number":49,"context_line":"            pgsql_service \u003d operating_system.service_discovery("},{"line_number":50,"context_line":"                PGSQL_SERVICE_CANDIDATES)"},{"line_number":51,"context_line":"            utils.execute_with_timeout(pgsql_service[\u0027cmd_enable\u0027],"},{"line_number":52,"context_line":"                                       shell\u003dTrue)"},{"line_number":53,"context_line":"        except KeyError:"},{"line_number":54,"context_line":"            LOG.exception(_(\"Error enabling PostgreSQL start on boot.\"))"},{"line_number":55,"context_line":"            raise RuntimeError(\"Service is not discovered.\")"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa81d914_f7f0fcff","line":52,"in_reply_to":"fa81d914_88aec5e4","updated":"2015-01-30 13:31:01.000000000","message":"If I\u0027m reading the code correctly, I see the following command for cmd_enable under the sysvinit section of operating_system.service_discovery:\n\n  result[\u0027cmd_enable\u0027] \u003d \"sudo update-rc.d %s defaults; sudo \" \\\n                                       \"update-rc.d %s enable\" % (service,\n                                                                  service)\n\nThia would require shell\u003dTrue for the second command to execute.\n\nI also see:\n\n  result[\u0027cmd_start\u0027] \u003d \"sudo start %s || true\" % service\n\nin the upstart section, also requiring shell\u003dTrue (although _enable_pgsql_on_boot is clearly running cmd_enable, not cmd_start)\n\nMaybe I\u0027m reading the code wrong ...","commit_id":"9bfbcaf159f000759646ef93966a25533a7b0ba2"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"3ae6ec3d9eb6ccc64c66fa38a0bbf84b859bd178","unresolved":false,"context_lines":[{"line_number":49,"context_line":"            pgsql_service \u003d operating_system.service_discovery("},{"line_number":50,"context_line":"                PGSQL_SERVICE_CANDIDATES)"},{"line_number":51,"context_line":"            utils.execute_with_timeout(pgsql_service[\u0027cmd_enable\u0027],"},{"line_number":52,"context_line":"                                       shell\u003dTrue)"},{"line_number":53,"context_line":"        except KeyError:"},{"line_number":54,"context_line":"            LOG.exception(_(\"Error enabling PostgreSQL start on boot.\"))"},{"line_number":55,"context_line":"            raise RuntimeError(\"Service is not discovered.\")"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa81d914_88aec5e4","line":52,"in_reply_to":"fa81d914_e45d1d4a","updated":"2015-01-30 10:00:55.000000000","message":"I believe the only commands that we would be running in this case (service discovery) fall into one of the following 3 categories:\n- upstart (eg. \"start \u003cservice\u003e\")\n- sysvinit (eg. \"service \u003cservice\u003e start\")\n- systemd (eg. \"systemctl start \u003cservice\u003e\")\n\nNone of these should need pipes or redirection, requiring shell\u003dTrue.","commit_id":"9bfbcaf159f000759646ef93966a25533a7b0ba2"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"6fcaaa9c0810190dd81a3f8feca28af503aff36f","unresolved":false,"context_lines":[{"line_number":49,"context_line":"            pgsql_service \u003d operating_system.service_discovery("},{"line_number":50,"context_line":"                PGSQL_SERVICE_CANDIDATES)"},{"line_number":51,"context_line":"            utils.execute_with_timeout(pgsql_service[\u0027cmd_enable\u0027],"},{"line_number":52,"context_line":"                                       shell\u003dTrue)"},{"line_number":53,"context_line":"        except KeyError:"},{"line_number":54,"context_line":"            LOG.exception(_(\"Error enabling PostgreSQL start on boot.\"))"},{"line_number":55,"context_line":"            raise RuntimeError(\"Service is not discovered.\")"}],"source_content_type":"text/x-python","patch_set":5,"id":"da86d52c_1aee0589","line":52,"in_reply_to":"fa81d914_f7f0fcff","updated":"2015-02-02 09:34:26.000000000","message":"Fair enough -- I missed the fact that cmd_enable for sysvinit was actually running two commands using \u0027;\u0027. Thanks for pointing that out.","commit_id":"9bfbcaf159f000759646ef93966a25533a7b0ba2"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"19fbcf10d825c913202a6fd8d7b716bdfa135c18","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    \"\"\"Mixin that manages the PgSql process.\"\"\""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    def start_db(self, context):"},{"line_number":32,"context_line":"        self._enable_pgsql_on_boot()"},{"line_number":33,"context_line":"        \"\"\"Start the PgSql service.\"\"\""},{"line_number":34,"context_line":"        cmd \u003d operating_system.service_discovery(PGSQL_SERVICE_CANDIDATES)"},{"line_number":35,"context_line":"        LOG.info("}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_14e7ca41","line":32,"updated":"2015-02-20 12:28:05.000000000","message":"Why?\n\nIsn\u0027t this doing more than just \u0027start the database\u0027?\n\nI wonder why you want to do this here.","commit_id":"69773141dd194a9c06685b86e2e88fd78d0bd314"},{"author":{"_account_id":8214,"name":"Auston McReynolds","email":"amcreynolds@ebaysf.com","username":"amcreynolds"},"change_message_id":"7c1d73e28830b3f25a10b56771672025c48a3c02","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    \"\"\"Mixin that manages the PgSql process.\"\"\""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    def start_db(self, context):"},{"line_number":32,"context_line":"        self._enable_pgsql_on_boot()"},{"line_number":33,"context_line":"        \"\"\"Start the PgSql service.\"\"\""},{"line_number":34,"context_line":"        cmd \u003d operating_system.service_discovery(PGSQL_SERVICE_CANDIDATES)"},{"line_number":35,"context_line":"        LOG.info("}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_1912e261","line":32,"in_reply_to":"ba7be1f8_14e7ca41","updated":"2015-02-26 01:16:38.000000000","message":"doesn\u0027t seem necessary. i think the inspiration was drawn from other datastores (i see the same logic for cassandra for ex).","commit_id":"69773141dd194a9c06685b86e2e88fd78d0bd314"},{"author":{"_account_id":14176,"name":"Nirav Shah","email":"nshah@tesora.com","username":"nshah"},"change_message_id":"19ea6c06e132da70dd1436433dd66247b07e633f","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    \"\"\"Mixin that manages the PgSql process.\"\"\""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    def start_db(self, context):"},{"line_number":32,"context_line":"        self._enable_pgsql_on_boot()"},{"line_number":33,"context_line":"        \"\"\"Start the PgSql service.\"\"\""},{"line_number":34,"context_line":"        cmd \u003d operating_system.service_discovery(PGSQL_SERVICE_CANDIDATES)"},{"line_number":35,"context_line":"        LOG.info("}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_e05ad4f0","line":32,"in_reply_to":"ba7be1f8_1912e261","updated":"2015-03-03 17:57:26.000000000","message":"As Auston correctly mentions, this was \u0027inspired\u0027 by other datastores. I wanted to keep this consistent with other datastore implementation.","commit_id":"69773141dd194a9c06685b86e2e88fd78d0bd314"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"19fbcf10d825c913202a6fd8d7b716bdfa135c18","unresolved":false,"context_lines":[{"line_number":50,"context_line":"                PGSQL_SERVICE_CANDIDATES)"},{"line_number":51,"context_line":"            utils.execute_with_timeout(pgsql_service[\u0027cmd_enable\u0027],"},{"line_number":52,"context_line":"                                       shell\u003dTrue)"},{"line_number":53,"context_line":"        except KeyError:"},{"line_number":54,"context_line":"            LOG.exception(_(\"Error enabling PostgreSQL start on boot.\"))"},{"line_number":55,"context_line":"            raise RuntimeError(\"Service is not discovered.\")"},{"line_number":56,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_d44a9226","line":53,"updated":"2015-02-20 12:28:05.000000000","message":"execute_with_timeout may also raise some exceptions. It appears that you are going to pass those on directly to the caller.","commit_id":"69773141dd194a9c06685b86e2e88fd78d0bd314"},{"author":{"_account_id":8214,"name":"Auston McReynolds","email":"amcreynolds@ebaysf.com","username":"amcreynolds"},"change_message_id":"7c1d73e28830b3f25a10b56771672025c48a3c02","unresolved":false,"context_lines":[{"line_number":50,"context_line":"                PGSQL_SERVICE_CANDIDATES)"},{"line_number":51,"context_line":"            utils.execute_with_timeout(pgsql_service[\u0027cmd_enable\u0027],"},{"line_number":52,"context_line":"                                       shell\u003dTrue)"},{"line_number":53,"context_line":"        except KeyError:"},{"line_number":54,"context_line":"            LOG.exception(_(\"Error enabling PostgreSQL start on boot.\"))"},{"line_number":55,"context_line":"            raise RuntimeError(\"Service is not discovered.\")"},{"line_number":56,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_dc29188b","line":53,"in_reply_to":"ba7be1f8_d44a9226","updated":"2015-02-26 01:16:38.000000000","message":"good catch, though i see this issue for other datastores as well.","commit_id":"69773141dd194a9c06685b86e2e88fd78d0bd314"},{"author":{"_account_id":14176,"name":"Nirav Shah","email":"nshah@tesora.com","username":"nshah"},"change_message_id":"19ea6c06e132da70dd1436433dd66247b07e633f","unresolved":false,"context_lines":[{"line_number":50,"context_line":"                PGSQL_SERVICE_CANDIDATES)"},{"line_number":51,"context_line":"            utils.execute_with_timeout(pgsql_service[\u0027cmd_enable\u0027],"},{"line_number":52,"context_line":"                                       shell\u003dTrue)"},{"line_number":53,"context_line":"        except KeyError:"},{"line_number":54,"context_line":"            LOG.exception(_(\"Error enabling PostgreSQL start on boot.\"))"},{"line_number":55,"context_line":"            raise RuntimeError(\"Service is not discovered.\")"},{"line_number":56,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_c07f9015","line":53,"in_reply_to":"ba7be1f8_dc29188b","updated":"2015-03-03 17:57:26.000000000","message":"This seems more like a generic issue that is prevalent in all datastores. \nIn both of these cases (this and start_db), I was trying to stay consistent with other datastores\u0027 implementations.","commit_id":"69773141dd194a9c06685b86e2e88fd78d0bd314"}]}