)]}'
{"nova/compute/api.py":[{"author":{"_account_id":1528,"name":"Stanislaw Pitucha","email":"viraptor@gmail.com","username":"stanislaw-pitucha"},"change_message_id":"66fb732ff6abe0bb206bfdcc2067c650ac613c80","unresolved":false,"context_lines":[{"line_number":608,"context_line":"                        reservation_id, scheduler_hints)"},{"line_number":609,"context_line":""},{"line_number":610,"context_line":"        for instance in instances:"},{"line_number":611,"context_line":"            event \u003d compute_utils.pack_action_start(context, instance[\u0027uuid\u0027],"},{"line_number":612,"context_line":"                                                    \u0027create\u0027)"},{"line_number":613,"context_line":"            self.db.action_start(context, event)"},{"line_number":614,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAM3%2F%2FfyI%3D","line":611,"updated":"2013-01-18 14:00:38.000000000","message":"I find the naming here a bit confusing. The variable is `event`, but it\u0027s actually an action, isn\u0027t it? The same name is used for storing results of pack_action_event_start(...) in another file. Is this misnamed, or am I just confused?","commit_id":"a7f1802a5dbbad5517ccfa218c8ebd4cde09566a"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"4abc09ce122fde5cc857bd5fdcb398227427a149","unresolved":false,"context_lines":[{"line_number":608,"context_line":"                        reservation_id, scheduler_hints)"},{"line_number":609,"context_line":""},{"line_number":610,"context_line":"        for instance in instances:"},{"line_number":611,"context_line":"            event \u003d compute_utils.pack_action_start(context, instance[\u0027uuid\u0027],"},{"line_number":612,"context_line":"                                                    \u0027create\u0027)"},{"line_number":613,"context_line":"            self.db.action_start(context, event)"},{"line_number":614,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAM3%2F%2Fd1E%3D","line":611,"in_reply_to":"AAAAM3%2F%2FfyI%3D","updated":"2013-01-21 14:43:54.000000000","message":"This is misnamed, I will fix it.","commit_id":"a7f1802a5dbbad5517ccfa218c8ebd4cde09566a"},{"author":{"_account_id":1528,"name":"Stanislaw Pitucha","email":"viraptor@gmail.com","username":"stanislaw-pitucha"},"change_message_id":"66fb732ff6abe0bb206bfdcc2067c650ac613c80","unresolved":false,"context_lines":[{"line_number":609,"context_line":""},{"line_number":610,"context_line":"        for instance in instances:"},{"line_number":611,"context_line":"            event \u003d compute_utils.pack_action_start(context, instance[\u0027uuid\u0027],"},{"line_number":612,"context_line":"                                                    \u0027create\u0027)"},{"line_number":613,"context_line":"            self.db.action_start(context, event)"},{"line_number":614,"context_line":""},{"line_number":615,"context_line":"        self.scheduler_rpcapi.run_instance(context,"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAM3%2F%2Ffx4%3D","line":612,"updated":"2013-01-18 14:00:38.000000000","message":"Is there an official list of names somewhere? Maybe it\u0027s possible to either use the current function\u0027s name instead (they seem to match in most cases and @wrap_istance_action does it), or create a module listing all possible names - same as vm_states does now? It would prevent silly misspelling that cause mismatch in the future.","commit_id":"a7f1802a5dbbad5517ccfa218c8ebd4cde09566a"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"4abc09ce122fde5cc857bd5fdcb398227427a149","unresolved":false,"context_lines":[{"line_number":609,"context_line":""},{"line_number":610,"context_line":"        for instance in instances:"},{"line_number":611,"context_line":"            event \u003d compute_utils.pack_action_start(context, instance[\u0027uuid\u0027],"},{"line_number":612,"context_line":"                                                    \u0027create\u0027)"},{"line_number":613,"context_line":"            self.db.action_start(context, event)"},{"line_number":614,"context_line":""},{"line_number":615,"context_line":"        self.scheduler_rpcapi.run_instance(context,"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAM3%2F%2Fd1A%3D","line":612,"in_reply_to":"AAAAM3%2F%2Ffx4%3D","updated":"2013-01-21 14:43:54.000000000","message":"Good idea.  I think an official list of names will work best.  I used the method name in @wrap_instance_action because it wrapped the public method, but now some things are being recorded in private helper methods so it\u0027s not always the best name to use.","commit_id":"a7f1802a5dbbad5517ccfa218c8ebd4cde09566a"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"56cb7ab0997d362e7128f418c91f2b6a26bf19bb","unresolved":false,"context_lines":[{"line_number":615,"context_line":"        for instance in instances:"},{"line_number":616,"context_line":"            action \u003d compute_utils.pack_action_start(context, instance[\u0027uuid\u0027],"},{"line_number":617,"context_line":"                                                     instance_actions.CREATE)"},{"line_number":618,"context_line":"            self.db.action_start(context, action)"},{"line_number":619,"context_line":""},{"line_number":620,"context_line":"        self.scheduler_rpcapi.run_instance(context,"},{"line_number":621,"context_line":"                request_spec\u003drequest_spec,"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAM3%2F%2FVNg%3D","line":618,"updated":"2013-01-28 18:39:31.000000000","message":"This compute_utils.pack_action_start()/db.action_start() idiom is repeated a lot.  Could you add a single internal method that performed this action, to DRY up the code?  Something like:\n\n    def _action_start(self, context, instance, action):\n        act \u003d compute_utils.pack_action_start(context, instance[\u0027uuid\u0027], action)\n        self.db.action_start(context, act)","commit_id":"99ac92bcc379e819b2f7baa6a55a0cbb1426d158"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"6f12a224fa7382cca702a899c378ffc44ed88bf4","unresolved":false,"context_lines":[{"line_number":615,"context_line":"        for instance in instances:"},{"line_number":616,"context_line":"            action \u003d compute_utils.pack_action_start(context, instance[\u0027uuid\u0027],"},{"line_number":617,"context_line":"                                                     instance_actions.CREATE)"},{"line_number":618,"context_line":"            self.db.action_start(context, action)"},{"line_number":619,"context_line":""},{"line_number":620,"context_line":"        self.scheduler_rpcapi.run_instance(context,"},{"line_number":621,"context_line":"                request_spec\u003drequest_spec,"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAM3%2F%2FVF0%3D","line":618,"in_reply_to":"AAAAM3%2F%2FVNg%3D","updated":"2013-01-28 20:07:58.000000000","message":"Certainly.  I started this way because I use the same idiom, except with the conductor api, in the compute manager so I couldn\u0027t refactor it to utils.  But I can DRY up both sides similarly.","commit_id":"99ac92bcc379e819b2f7baa6a55a0cbb1426d158"},{"author":{"_account_id":4419,"name":"Matthew Sherborne","email":"msherborne+openstack@gmail.com","username":"msherborne+openstack"},"change_message_id":"df1887876213ac1bc95850231b07f197abb09b26","unresolved":false,"context_lines":[{"line_number":194,"context_line":"    def _record_action_start(self, context, instance, action):"},{"line_number":195,"context_line":"        act \u003d compute_utils.pack_action_start(context, instance[\u0027uuid\u0027],"},{"line_number":196,"context_line":"                                              action)"},{"line_number":197,"context_line":"        self.db.action_start(context, act)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    def _check_injected_file_quota(self, context, injected_files):"},{"line_number":200,"context_line":"        \"\"\"Enforce quota limits on injected files."}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAM3%2F%2FSSc%3D","line":197,"updated":"2013-01-30 10:26:32.000000000","message":"Why not record using just rpc notifications and leave it out of nova proper?","commit_id":"a56144969c6e1177bfaf542b764e92cbecc6103b"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"857b366e6a3d2b64f51a7ea9b169bbecf109969e","unresolved":false,"context_lines":[{"line_number":194,"context_line":"    def _record_action_start(self, context, instance, action):"},{"line_number":195,"context_line":"        act \u003d compute_utils.pack_action_start(context, instance[\u0027uuid\u0027],"},{"line_number":196,"context_line":"                                              action)"},{"line_number":197,"context_line":"        self.db.action_start(context, act)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    def _check_injected_file_quota(self, context, injected_files):"},{"line_number":200,"context_line":"        \"\"\"Enforce quota limits on injected files."}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAM3%2F%2FR1U%3D","line":197,"in_reply_to":"AAAAM3%2F%2FSSc%3D","updated":"2013-01-30 15:26:19.000000000","message":"A few reasons: \n\n1. It\u0027s a lot easier to use if it doesn\u0027t require deploying a separate service.\n\n2. Moving in that direction gets into ceilometer territory which it\u0027s really not a good fit for.\n\n3. A big part of this is being able to provide information about instance errors through the API without having to set an instance to ERROR, and that can\u0027t be easily achieved through a separate service.","commit_id":"a56144969c6e1177bfaf542b764e92cbecc6103b"},{"author":{"_account_id":4419,"name":"Matthew Sherborne","email":"msherborne+openstack@gmail.com","username":"msherborne+openstack"},"change_message_id":"df1887876213ac1bc95850231b07f197abb09b26","unresolved":false,"context_lines":[{"line_number":1230,"context_line":"                    expected_task_state\u003dNone,"},{"line_number":1231,"context_line":"                    progress\u003d0)"},{"line_number":1232,"context_line":""},{"line_number":1233,"context_line":"        self._record_action_start(context, instance, instance_actions.STOP)"},{"line_number":1234,"context_line":""},{"line_number":1235,"context_line":"        self.compute_rpcapi.stop_instance(context, instance, cast\u003ddo_cast)"},{"line_number":1236,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAM3%2F%2FSRk%3D","line":1233,"updated":"2013-01-30 10:26:32.000000000","message":"This **may** generate a lot of DB records for some use cases. I\u0027d hit stop/start/reboot carefully.\n\nFor example where a user is testing something at boot. I can\u0027t think of anything likely to cause a hassle though, but might be something to keep in mind.\n\nPerhaps an option to turn on/off logging of start and stop.. or using the notifications interface and a plugin to that to record these events.","commit_id":"a56144969c6e1177bfaf542b764e92cbecc6103b"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"857b366e6a3d2b64f51a7ea9b169bbecf109969e","unresolved":false,"context_lines":[{"line_number":1230,"context_line":"                    expected_task_state\u003dNone,"},{"line_number":1231,"context_line":"                    progress\u003d0)"},{"line_number":1232,"context_line":""},{"line_number":1233,"context_line":"        self._record_action_start(context, instance, instance_actions.STOP)"},{"line_number":1234,"context_line":""},{"line_number":1235,"context_line":"        self.compute_rpcapi.stop_instance(context, instance, cast\u003ddo_cast)"},{"line_number":1236,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAM3%2F%2FRxw%3D","line":1233,"in_reply_to":"AAAAM3%2F%2FSRk%3D","updated":"2013-01-30 15:26:19.000000000","message":"If this looks like it can become a performance bottleneck I will add some sort of toggle.  I\u0027m more concerned with expiring records when a certain number is reached for an instance, and cleanup when instances are deleted as I seem to recall some mention of eventually doing something with those records to keep the db from getting huge.  But I will look to address these concerns in a future patch.","commit_id":"a56144969c6e1177bfaf542b764e92cbecc6103b"}],"nova/compute/instance_actions.py":[{"author":{"_account_id":360,"name":"Dan Prince","email":"dprince@redhat.com","username":"dan-prince"},"change_message_id":"6cbb9e695298d5b4f175ca880d35f853da7f2ebd","unresolved":false,"context_lines":[{"line_number":32,"context_line":"START \u003d \u0027start\u0027"},{"line_number":33,"context_line":"REBOOT \u003d \u0027reboot\u0027"},{"line_number":34,"context_line":"REBUILD \u003d \u0027rebuild\u0027"},{"line_number":35,"context_line":"REVERTRESIZE \u003d \u0027revertResize\u0027"},{"line_number":36,"context_line":"CONFIRMRESIZE \u003d \u0027confirmResize\u0027"},{"line_number":37,"context_line":"RESIZE \u003d \u0027resize\u0027"},{"line_number":38,"context_line":"PAUSE \u003d \u0027pause\u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAM3%2F%2FTDE%3D","line":35,"updated":"2013-01-29 20:53:42.000000000","message":"How would you feel about making these REVERT_RESIZE and CONFIRM_RESIZE?","commit_id":"1658e666b80a71fc8e6545d0099ceda81674aaf3"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"d685de3283ce6d0ad6328ec26c2ac895c63da908","unresolved":false,"context_lines":[{"line_number":32,"context_line":"START \u003d \u0027start\u0027"},{"line_number":33,"context_line":"REBOOT \u003d \u0027reboot\u0027"},{"line_number":34,"context_line":"REBUILD \u003d \u0027rebuild\u0027"},{"line_number":35,"context_line":"REVERTRESIZE \u003d \u0027revertResize\u0027"},{"line_number":36,"context_line":"CONFIRMRESIZE \u003d \u0027confirmResize\u0027"},{"line_number":37,"context_line":"RESIZE \u003d \u0027resize\u0027"},{"line_number":38,"context_line":"PAUSE \u003d \u0027pause\u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAM3%2F%2FTBM%3D","line":35,"in_reply_to":"AAAAM3%2F%2FTDE%3D","updated":"2013-01-29 21:02:18.000000000","message":"Works for me.","commit_id":"1658e666b80a71fc8e6545d0099ceda81674aaf3"},{"author":{"_account_id":360,"name":"Dan Prince","email":"dprince@redhat.com","username":"dan-prince"},"change_message_id":"6cbb9e695298d5b4f175ca880d35f853da7f2ebd","unresolved":false,"context_lines":[{"line_number":41,"context_line":"RESUME \u003d \u0027resume\u0027"},{"line_number":42,"context_line":"RESCUE \u003d \u0027rescue\u0027"},{"line_number":43,"context_line":"UNRESCUE \u003d \u0027unrescue\u0027"},{"line_number":44,"context_line":"CHANGEPASSWORD \u003d \u0027changePassword\u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAM3%2F%2FTC8%3D","line":44,"updated":"2013-01-29 20:53:42.000000000","message":"Ditto here: CHANGE_PASSWORD?","commit_id":"1658e666b80a71fc8e6545d0099ceda81674aaf3"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"d685de3283ce6d0ad6328ec26c2ac895c63da908","unresolved":false,"context_lines":[{"line_number":41,"context_line":"RESUME \u003d \u0027resume\u0027"},{"line_number":42,"context_line":"RESCUE \u003d \u0027rescue\u0027"},{"line_number":43,"context_line":"UNRESCUE \u003d \u0027unrescue\u0027"},{"line_number":44,"context_line":"CHANGEPASSWORD \u003d \u0027changePassword\u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAM3%2F%2FTBE%3D","line":44,"in_reply_to":"AAAAM3%2F%2FTC8%3D","updated":"2013-01-29 21:02:18.000000000","message":"Same.","commit_id":"1658e666b80a71fc8e6545d0099ceda81674aaf3"}],"nova/compute/manager.py":[{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"763013cc8f160c36cc4146dd030dda8f7e10b677","unresolved":false,"context_lines":[{"line_number":244,"context_line":""},{"line_number":245,"context_line":"    @functools.wraps(function)"},{"line_number":246,"context_line":"    def decorated_function(self, context, *args, **kwargs):"},{"line_number":247,"context_line":"        #NOTE(alaski): If instance is passed as an arg, rather than kwarg,"},{"line_number":248,"context_line":"        # there\u0027s no guarantee on its position.  So we\u0027ll skip recording the"},{"line_number":249,"context_line":"        # event for it."},{"line_number":250,"context_line":"        if \u0027instance\u0027 in kwargs:"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAM3%2F%2FYyU%3D","line":247,"updated":"2013-01-24 15:37:17.000000000","message":"Which methods actually pass instance as a kwarg?  I\u0027m not seeing any.\n\nCheck out the inspect module\u0027s getargspec().  You can get the actual name of the argument.  While slightly hackish, I think looking for an argument named instance is pragmatic here. :)","commit_id":"a7f1802a5dbbad5517ccfa218c8ebd4cde09566a"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"28b3fb916a20d4a9f4fda516d96917dab78be932","unresolved":false,"context_lines":[{"line_number":244,"context_line":""},{"line_number":245,"context_line":"    @functools.wraps(function)"},{"line_number":246,"context_line":"    def decorated_function(self, context, *args, **kwargs):"},{"line_number":247,"context_line":"        #NOTE(alaski): If instance is passed as an arg, rather than kwarg,"},{"line_number":248,"context_line":"        # there\u0027s no guarantee on its position.  So we\u0027ll skip recording the"},{"line_number":249,"context_line":"        # event for it."},{"line_number":250,"context_line":"        if \u0027instance\u0027 in kwargs:"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAM3%2F%2FYZg%3D","line":247,"in_reply_to":"AAAAM3%2F%2FYyU%3D","updated":"2013-01-24 18:32:35.000000000","message":"None do in this patch.  But some of the tests in nova/tests/compute/test_compute.py were.  I mainly left the note as a warning that there\u0027s not an easy solution to this in case it breaks in the future and someone tries to look for it positionally.\n\nI\u0027m happy to solve it that way as well, check out https://review.openstack.org/#/c/17716/4/nova/utils.py which is what I needed to get at instance reliably.  It does use getargspec.  I\u0027ll go with whatever method causes the least heartburn for people, because both methods have drawbacks.  Though I will admit I\u0027m somewhat proud of the hacky way :)","commit_id":"a7f1802a5dbbad5517ccfa218c8ebd4cde09566a"},{"author":{"_account_id":100,"name":"Johannes Erdfelt","email":"johannes@erdfelt.com","username":"johannes.erdfelt"},"change_message_id":"bc90b35d374bf3d2f699cf757f503bd2972d1dc5","unresolved":false,"context_lines":[{"line_number":233,"context_line":"    return decorated_function"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"def wrap_instance_event(function):"},{"line_number":237,"context_line":"    \"\"\"Wraps a method to log the event taken on the instance, and result."},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"    This decorator wraps a method to log the start and result of an event, as"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAM3%2F%2FPQg%3D","line":236,"updated":"2013-01-31 21:17:59.000000000","message":"I haven\u0027t audited the code, but is there ever a case where wrap_instance_fault is used but wrap_instance_event wouldn\u0027t be?\n\nIf not, would it make more sense to merge these two? It would make the code cleaner then and require one less decorator","commit_id":"a56144969c6e1177bfaf542b764e92cbecc6103b"},{"author":{"_account_id":4419,"name":"Matthew Sherborne","email":"msherborne+openstack@gmail.com","username":"msherborne+openstack"},"change_message_id":"26ca82c15ad1e44515d360832700b1ca168f2b62","unresolved":false,"context_lines":[{"line_number":233,"context_line":"    return decorated_function"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"def wrap_instance_event(function):"},{"line_number":237,"context_line":"    \"\"\"Wraps a method to log the event taken on the instance, and result."},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"    This decorator wraps a method to log the start and result of an event, as"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAM3%2F%2FO7k%3D","line":236,"in_reply_to":"AAAAM3%2F%2FPKI%3D","updated":"2013-02-01 01:09:14.000000000","message":"It\u0027s good to have them separate.\n\nThey do different things, and are run in different circumstances; they\u0027re each part of separate systems. Better to be explicit than try to force merge code that doesn\u0027t really want to be.\n\nI was concerned maybe that the order of including the decorators might change the behaviour, but from my examination the order looks to be unimportant.","commit_id":"a56144969c6e1177bfaf542b764e92cbecc6103b"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"d3f60b1ab9155b61a817b82233f5ffc41d620b14","unresolved":false,"context_lines":[{"line_number":233,"context_line":"    return decorated_function"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"def wrap_instance_event(function):"},{"line_number":237,"context_line":"    \"\"\"Wraps a method to log the event taken on the instance, and result."},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"    This decorator wraps a method to log the start and result of an event, as"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAM3%2F%2FPKI%3D","line":236,"in_reply_to":"AAAAM3%2F%2FPQg%3D","updated":"2013-01-31 21:53:53.000000000","message":"There are a few cases.  And there are two reasons for leaving it off there, neither of which prevent merging the two from a technical standpoint.\n\n1. Logging an event requires attaching it to an action from the API, and I didn\u0027t record an action for all API requests, specifically missing are some of the fixed_ip calls.  This is easy enough to fix in a couple of ways.\n\n2. There are some methods that I thought would cause too much noise if they were recorded.  Things like getting a console or getting console output I just didn\u0027t see as being helpful to record.\n\nSo with that said, are you still in favor of merging the two?","commit_id":"a56144969c6e1177bfaf542b764e92cbecc6103b"},{"author":{"_account_id":4419,"name":"Matthew Sherborne","email":"msherborne+openstack@gmail.com","username":"msherborne+openstack"},"change_message_id":"df1887876213ac1bc95850231b07f197abb09b26","unresolved":false,"context_lines":[{"line_number":234,"context_line":""},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"def wrap_instance_event(function):"},{"line_number":237,"context_line":"    \"\"\"Wraps a method to log the event taken on the instance, and result."},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"    This decorator wraps a method to log the start and result of an event, as"},{"line_number":240,"context_line":"    part of an action taken on an instance."}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAM3%2F%2FSPE%3D","line":237,"updated":"2013-01-30 10:26:32.000000000","message":"I\u0027d drop the comma. \u003c/pedantic\u003e","commit_id":"a56144969c6e1177bfaf542b764e92cbecc6103b"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"857b366e6a3d2b64f51a7ea9b169bbecf109969e","unresolved":false,"context_lines":[{"line_number":234,"context_line":""},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"def wrap_instance_event(function):"},{"line_number":237,"context_line":"    \"\"\"Wraps a method to log the event taken on the instance, and result."},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"    This decorator wraps a method to log the start and result of an event, as"},{"line_number":240,"context_line":"    part of an action taken on an instance."}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAM3%2F%2FRxg%3D","line":237,"in_reply_to":"AAAAM3%2F%2FSPE%3D","updated":"2013-01-30 15:26:19.000000000","message":"If I end up pushing another patchset I can change it.  When I think this sentence to myself there\u0027s a pause there.","commit_id":"a56144969c6e1177bfaf542b764e92cbecc6103b"},{"author":{"_account_id":100,"name":"Johannes Erdfelt","email":"johannes@erdfelt.com","username":"johannes.erdfelt"},"change_message_id":"bc90b35d374bf3d2f699cf757f503bd2972d1dc5","unresolved":false,"context_lines":[{"line_number":244,"context_line":"    def decorated_function(self, context, *args, **kwargs):"},{"line_number":245,"context_line":"        #NOTE(alaski): If instance is passed as an arg, rather than kwarg,"},{"line_number":246,"context_line":"        # there\u0027s no guarantee on its position.  So we\u0027ll skip recording the"},{"line_number":247,"context_line":"        # event for it."},{"line_number":248,"context_line":"        if \u0027instance\u0027 in kwargs:"},{"line_number":249,"context_line":"            instance_uuid \u003d kwargs[\u0027instance\u0027][\u0027uuid\u0027]"},{"line_number":250,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAM3%2F%2FPQE%3D","line":247,"updated":"2013-01-31 21:17:59.000000000","message":"Why not use the same implementation that wrap_instance_fault uses then? (Also another reason to merge these two decorators)","commit_id":"a56144969c6e1177bfaf542b764e92cbecc6103b"},{"author":{"_account_id":4419,"name":"Matthew Sherborne","email":"msherborne+openstack@gmail.com","username":"msherborne+openstack"},"change_message_id":"26ca82c15ad1e44515d360832700b1ca168f2b62","unresolved":false,"context_lines":[{"line_number":244,"context_line":"    def decorated_function(self, context, *args, **kwargs):"},{"line_number":245,"context_line":"        #NOTE(alaski): If instance is passed as an arg, rather than kwarg,"},{"line_number":246,"context_line":"        # there\u0027s no guarantee on its position.  So we\u0027ll skip recording the"},{"line_number":247,"context_line":"        # event for it."},{"line_number":248,"context_line":"        if \u0027instance\u0027 in kwargs:"},{"line_number":249,"context_line":"            instance_uuid \u003d kwargs[\u0027instance\u0027][\u0027uuid\u0027]"},{"line_number":250,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAM3%2F%2FO4o%3D","line":247,"in_reply_to":"AAAAM3%2F%2FPE8%3D","updated":"2013-02-01 01:09:14.000000000","message":"I vote option 2.\n\nI\u0027m tempted to leave it as it is, but choosing option 2 early will help preempt future bugs/complaints. It might be a bit hard to trace the reason it\u0027s not recording the event and create unnecessary work.\n\nI don\u0027t see anything any more freaky in get_wrapped_function, than the rest of the nova code base. So I don\u0027t see any downside to option 2.","commit_id":"a56144969c6e1177bfaf542b764e92cbecc6103b"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"2f66a1951c595c8bc3b9d3973f7c61bbdc87925f","unresolved":false,"context_lines":[{"line_number":244,"context_line":"    def decorated_function(self, context, *args, **kwargs):"},{"line_number":245,"context_line":"        #NOTE(alaski): If instance is passed as an arg, rather than kwarg,"},{"line_number":246,"context_line":"        # there\u0027s no guarantee on its position.  So we\u0027ll skip recording the"},{"line_number":247,"context_line":"        # event for it."},{"line_number":248,"context_line":"        if \u0027instance\u0027 in kwargs:"},{"line_number":249,"context_line":"            instance_uuid \u003d kwargs[\u0027instance\u0027][\u0027uuid\u0027]"},{"line_number":250,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAM3%2F%2FPE8%3D","line":247,"in_reply_to":"AAAAM3%2F%2FPHU%3D","updated":"2013-01-31 22:33:44.000000000","message":"How do you feel about lines 244-245 in https://review.openstack.org/#/c/17716/1/nova/compute/manager.py with their implementation at https://review.openstack.org/#/c/17716/1/nova/utils.py?  It\u0027s a much more robust version of what line 226 is doing above.\n\nI\u0027m concerned about the fragility as well but I see three choices:\n\n1. Keep using this.  It\u0027s fragile but shouldn\u0027t lead to failure if something goes wrong, it just won\u0027t record an event.\n\n2. Use what I linked above.  As far as I can tell it works in all current and hopefully future cases, is well tested, and allows for args, kwargs, whatever.  The downside is that the get_wrapped_function() method is something that would make my heart race if I stumbled upon it.  Though I\u0027m proud of that code and the testing around it.\n\n3. Merge with wrap_instance_faults.  This adds some extra events I wasn\u0027t planning to record, and requires moving line 226 out of the exception path and into the regular path.  It\u0027s kind of a watered down version of option 2.\n\nI\u0027m happy to go any direction, though 3 is my least favorite.","commit_id":"a56144969c6e1177bfaf542b764e92cbecc6103b"},{"author":{"_account_id":100,"name":"Johannes Erdfelt","email":"johannes@erdfelt.com","username":"johannes.erdfelt"},"change_message_id":"954329f12cbfbf2635d8bf331920b39d0c31d144","unresolved":false,"context_lines":[{"line_number":244,"context_line":"    def decorated_function(self, context, *args, **kwargs):"},{"line_number":245,"context_line":"        #NOTE(alaski): If instance is passed as an arg, rather than kwarg,"},{"line_number":246,"context_line":"        # there\u0027s no guarantee on its position.  So we\u0027ll skip recording the"},{"line_number":247,"context_line":"        # event for it."},{"line_number":248,"context_line":"        if \u0027instance\u0027 in kwargs:"},{"line_number":249,"context_line":"            instance_uuid \u003d kwargs[\u0027instance\u0027][\u0027uuid\u0027]"},{"line_number":250,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAM3%2F%2FPHU%3D","line":247,"in_reply_to":"AAAAM3%2F%2FPN0%3D","updated":"2013-01-31 22:08:27.000000000","message":"I\u0027m concerned about the fact the functions need to be called with keyword arguments now even if positional arguments work fine. It requires a handful of seemingly unnecessary changes in your patch but also requires future programmers and reviewers get it right too.\n\nThis is especially bad because this only matters in the exception path and that\u0027s the poorest tested code path both in our unit tests but also in various other functional tests.\n\nI understand the technical reason now (missed the difference in ordering) but I\u0027m worried about how fragile this in the face of future changes.","commit_id":"a56144969c6e1177bfaf542b764e92cbecc6103b"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"d3f60b1ab9155b61a817b82233f5ffc41d620b14","unresolved":false,"context_lines":[{"line_number":244,"context_line":"    def decorated_function(self, context, *args, **kwargs):"},{"line_number":245,"context_line":"        #NOTE(alaski): If instance is passed as an arg, rather than kwarg,"},{"line_number":246,"context_line":"        # there\u0027s no guarantee on its position.  So we\u0027ll skip recording the"},{"line_number":247,"context_line":"        # event for it."},{"line_number":248,"context_line":"        if \u0027instance\u0027 in kwargs:"},{"line_number":249,"context_line":"            instance_uuid \u003d kwargs[\u0027instance\u0027][\u0027uuid\u0027]"},{"line_number":250,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAM3%2F%2FPN0%3D","line":247,"in_reply_to":"AAAAM3%2F%2FPQE%3D","updated":"2013-01-31 21:53:53.000000000","message":"That implementation only works because it\u0027s the wrapper closest to the wrapped method.  I exploited some meta programming to get at instance reliably in some earlier patchsets, but it\u0027s \u0027magic\u0027 and this is probably safer.","commit_id":"a56144969c6e1177bfaf542b764e92cbecc6103b"}],"nova/compute/utils.py":[{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"56cb7ab0997d362e7128f418c91f2b6a26bf19bb","unresolved":false,"context_lines":[{"line_number":291,"context_line":""},{"line_number":292,"context_line":""},{"line_number":293,"context_line":"class EventReporter(object):"},{"line_number":294,"context_line":"    \"\"\"Context manager to reporting instance action events.\"\"\""},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"    def __init__(self, context, event_name, *instance_uuids):"},{"line_number":297,"context_line":"        self.context \u003d context"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAM3%2F%2FVNQ%3D","line":294,"updated":"2013-01-28 18:39:31.000000000","message":"s/reporting/report/","commit_id":"99ac92bcc379e819b2f7baa6a55a0cbb1426d158"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d0cf83d11f9c206f0851389c196076974163ccf1","unresolved":false,"context_lines":[{"line_number":302,"context_line":"        for uuid in self.instance_uuids:"},{"line_number":303,"context_line":"            event \u003d pack_action_event_start(self.context, uuid,"},{"line_number":304,"context_line":"                                            self.event_name)"},{"line_number":305,"context_line":"            db.action_event_start(self.context, event)"},{"line_number":306,"context_line":""},{"line_number":307,"context_line":"        return self"},{"line_number":308,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAM3%2F%2FU0Q%3D","line":305,"updated":"2013-01-28 22:28:48.000000000","message":"I wanted to make sure that these direct database calls aren\u0027t being made from n-cpu, but I couldn\u0027t find where this EventReporter is used. Could you help me out?","commit_id":"1658e666b80a71fc8e6545d0099ceda81674aaf3"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"de498e6e7394435971fea94495ff7ebe3a8eeec1","unresolved":false,"context_lines":[{"line_number":302,"context_line":"        for uuid in self.instance_uuids:"},{"line_number":303,"context_line":"            event \u003d pack_action_event_start(self.context, uuid,"},{"line_number":304,"context_line":"                                            self.event_name)"},{"line_number":305,"context_line":"            db.action_event_start(self.context, event)"},{"line_number":306,"context_line":""},{"line_number":307,"context_line":"        return self"},{"line_number":308,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAM3%2F%2FT9g%3D","line":305,"in_reply_to":"AAAAM3%2F%2FU0Q%3D","updated":"2013-01-29 14:16:03.000000000","message":"Certainly.  It\u0027s used in the scheduler manager, https://review.openstack.org/#/c/17716/10/nova/scheduler/manager.py lines 107 and 133.  The compute code is using the conductor service.\n\nAs a return question, I could re-use this context manager on the compute side if I passed in the db or conductor from the caller.  But I didn\u0027t want to go down that path as I haven\u0027t heard anything around maintaining compatibility between those apis.  Is that something that has been considered?","commit_id":"1658e666b80a71fc8e6545d0099ceda81674aaf3"}],"nova/conductor/manager.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f46378fc4b20b4231653993f24adee1cc36b02f1","unresolved":false,"context_lines":[{"line_number":266,"context_line":"            if key in datetime_fields and isinstance(value, basestring):"},{"line_number":267,"context_line":"                values[key] \u003d timeutils.parse_strtime(value)"},{"line_number":268,"context_line":"        result \u003d self.db.action_event_start(context, values)"},{"line_number":269,"context_line":"        return jsonutils.to_primitive(result)"},{"line_number":270,"context_line":""},{"line_number":271,"context_line":"    def action_event_finish(self, context, values):"},{"line_number":272,"context_line":"        for key, value in values.iteritems():"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAM3%2F%2FoDA%3D","line":269,"updated":"2013-01-12 01:16:45.000000000","message":"I\u0027m confused. If this is necessary, wouldn\u0027t every attempt to update instance timestamps fail when going through conductor?\n\nAnd if this is really the right thing to do, I think it needs to be captured in a from_primitive() sort of helper method, not spelled out in each method independently.","commit_id":"633fd481a183ddca37b36db4f6fcb546ad812655"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"60a3d0361676c0ae6930f7814f52c538fdbd687f","unresolved":false,"context_lines":[{"line_number":266,"context_line":"            if key in datetime_fields and isinstance(value, basestring):"},{"line_number":267,"context_line":"                values[key] \u003d timeutils.parse_strtime(value)"},{"line_number":268,"context_line":"        result \u003d self.db.action_event_start(context, values)"},{"line_number":269,"context_line":"        return jsonutils.to_primitive(result)"},{"line_number":270,"context_line":""},{"line_number":271,"context_line":"    def action_event_finish(self, context, values):"},{"line_number":272,"context_line":"        for key, value in values.iteritems():"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAM3%2F%2FmGs%3D","line":269,"in_reply_to":"AAAAM3%2F%2FnqY%3D","updated":"2013-01-14 18:24:48.000000000","message":"Sounds good.  Moved to the db api.  I left the datetime objects in the model returned by the db api for now but can convert those as well.","commit_id":"633fd481a183ddca37b36db4f6fcb546ad812655"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d0d671f0ce1914d8aff4c26d78baad0fe9cb574f","unresolved":false,"context_lines":[{"line_number":266,"context_line":"            if key in datetime_fields and isinstance(value, basestring):"},{"line_number":267,"context_line":"                values[key] \u003d timeutils.parse_strtime(value)"},{"line_number":268,"context_line":"        result \u003d self.db.action_event_start(context, values)"},{"line_number":269,"context_line":"        return jsonutils.to_primitive(result)"},{"line_number":270,"context_line":""},{"line_number":271,"context_line":"    def action_event_finish(self, context, values):"},{"line_number":272,"context_line":"        for key, value in values.iteritems():"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAM3%2F%2FnqY%3D","line":269,"in_reply_to":"AAAAM3%2F%2Fnrk%3D","updated":"2013-01-12 17:24:23.000000000","message":"I wouldn\u0027t be opposed to just putting this in the db api itself, for what it\u0027s worth. We\u0027re shooting to be using primitives everywhere except inside the db api, so making it tolerant of that seems like a good thing to me.\n\nI think all you need to do is check to see if something is a string or a datetime. If the former, try to parse it as the PERFECT_TIME format, and fail if that doesn\u0027t work (since it\u0027s clearly not valid) and obviously do nothing different if it\u0027s the latter.\n\nThe hunt-and-peck approach that would be done in conductor, either locally as you have it, or globally in a filter method, seems less clear and specific than an explicit approach in the db api.","commit_id":"633fd481a183ddca37b36db4f6fcb546ad812655"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"ecb4adfee142d35e6f9d2b378ace955650d4648c","unresolved":false,"context_lines":[{"line_number":266,"context_line":"            if key in datetime_fields and isinstance(value, basestring):"},{"line_number":267,"context_line":"                values[key] \u003d timeutils.parse_strtime(value)"},{"line_number":268,"context_line":"        result \u003d self.db.action_event_start(context, values)"},{"line_number":269,"context_line":"        return jsonutils.to_primitive(result)"},{"line_number":270,"context_line":""},{"line_number":271,"context_line":"    def action_event_finish(self, context, values):"},{"line_number":272,"context_line":"        for key, value in values.iteritems():"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAM3%2F%2Fnrk%3D","line":269,"in_reply_to":"AAAAM3%2F%2FoDA%3D","updated":"2013-01-12 16:36:01.000000000","message":"There are surprisingly few timestamp updates in these calls.  The only other one I found is on line 65 which handles it the same way I do it here, because I copied this from there.  I don\u0027t know how other db backends will handle it but sqlite is unhappy with putting a string into a datetime column, so at least for testing it is necessary.\n\nI\u0027ll try to put together a from_primitive method, though at the moment I\u0027m not sure how to determine what to cast back to reliably without passing further information.  Checking key names against a list or using regexes might get us there for now though.\n\nThanks for engaging me on this, I\u0027m still learning the intricacies of the conductor service and want to make things better and not worse.","commit_id":"633fd481a183ddca37b36db4f6fcb546ad812655"}],"nova/conductor/rpcapi.py":[{"author":{"_account_id":4419,"name":"Matthew Sherborne","email":"msherborne+openstack@gmail.com","username":"msherborne+openstack"},"change_message_id":"1e4d0fb415091e22b0a6074007aff9d9e3766c9f","unresolved":false,"context_lines":[{"line_number":299,"context_line":"        return self.call(context, msg, version\u003d\u00271.36\u0027)"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    def action_event_start(self, context, values):"},{"line_number":302,"context_line":"        values_p \u003d jsonutils.to_primitive(values)"},{"line_number":303,"context_line":"        msg \u003d self.make_msg(\u0027action_event_start\u0027, values\u003dvalues_p)"},{"line_number":304,"context_line":"        return self.call(context, msg, version\u003d\u00271.25\u0027)"},{"line_number":305,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAM3%2F%2FQYs%3D","line":302,"updated":"2013-01-31 06:19:08.000000000","message":"What\u0027s going on here ?","commit_id":"a56144969c6e1177bfaf542b764e92cbecc6103b"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"44a9308cbf89295ca65a61085cd42b2b8f05611f","unresolved":false,"context_lines":[{"line_number":299,"context_line":"        return self.call(context, msg, version\u003d\u00271.36\u0027)"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    def action_event_start(self, context, values):"},{"line_number":302,"context_line":"        values_p \u003d jsonutils.to_primitive(values)"},{"line_number":303,"context_line":"        msg \u003d self.make_msg(\u0027action_event_start\u0027, values\u003dvalues_p)"},{"line_number":304,"context_line":"        return self.call(context, msg, version\u003d\u00271.25\u0027)"},{"line_number":305,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAM3%2F%2FP18%3D","line":302,"in_reply_to":"AAAAM3%2F%2FQYs%3D","updated":"2013-01-31 16:25:10.000000000","message":"There\u0027s a datetime object in values, which need to be converted to a string before it can be passed over RPC.  I\u0027m using the same idiom that you\u0027ll find elsewhere in the file.","commit_id":"a56144969c6e1177bfaf542b764e92cbecc6103b"}],"nova/db/sqlalchemy/api.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"887a3a2d2788935398c7ec13724786b3e6936840","unresolved":false,"context_lines":[{"line_number":4576,"context_line":"def action_start(context, values):"},{"line_number":4577,"context_line":"    if isinstance(values[\u0027start_time\u0027], str):"},{"line_number":4578,"context_line":"        #NOTE(alaski): Don\u0027t catch an exception here because we can\u0027t recover."},{"line_number":4579,"context_line":"        values[\u0027start_time\u0027] \u003d timeutils.parse_strtime(values[\u0027start_time\u0027])"},{"line_number":4580,"context_line":""},{"line_number":4581,"context_line":"    action_ref \u003d models.InstanceAction()"},{"line_number":4582,"context_line":"    action_ref.update(values)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAM3%2F%2FkjM%3D","line":4579,"updated":"2013-01-15 17:11:43.000000000","message":"I hate to keep beating you up over this, but:\n\nI\u0027d really like to see this as a helper method in this file. I\u0027m working on something now where I\u0027ll need such a method in the same way you do here. I\u0027d like the above two lines to look something like:\n\nvalues \u003d convert_datetimes(values, \"start_time\")\n\nWhat do you think?","commit_id":"6937e96682c6b1d09e1d5bc4ee8129c4304d114f"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"5ef055fb9b295416cbcf7287b3d507111e60fe38","unresolved":false,"context_lines":[{"line_number":4576,"context_line":"def action_start(context, values):"},{"line_number":4577,"context_line":"    if isinstance(values[\u0027start_time\u0027], str):"},{"line_number":4578,"context_line":"        #NOTE(alaski): Don\u0027t catch an exception here because we can\u0027t recover."},{"line_number":4579,"context_line":"        values[\u0027start_time\u0027] \u003d timeutils.parse_strtime(values[\u0027start_time\u0027])"},{"line_number":4580,"context_line":""},{"line_number":4581,"context_line":"    action_ref \u003d models.InstanceAction()"},{"line_number":4582,"context_line":"    action_ref.update(values)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAM3%2F%2FkUA%3D","line":4579,"in_reply_to":"AAAAM3%2F%2FkjM%3D","updated":"2013-01-15 19:15:56.000000000","message":"I don\u0027t feel beat up, keep the suggestions coming.\n\nMeant to put a TODO about a utility method, because I wanted to ask about placement before putting something together.  I\u0027m happy to throw something in this file though.  I\u0027m assuming you see the second param as a string or list, is that correct?","commit_id":"6937e96682c6b1d09e1d5bc4ee8129c4304d114f"}],"nova/db/sqlalchemy/models.py":[{"author":{"_account_id":100,"name":"Johannes Erdfelt","email":"johannes@erdfelt.com","username":"johannes.erdfelt"},"change_message_id":"bc90b35d374bf3d2f699cf757f503bd2972d1dc5","unresolved":false,"context_lines":[{"line_number":1002,"context_line":"    \"\"\"Track client actions on an instance."},{"line_number":1003,"context_line":""},{"line_number":1004,"context_line":"    The intention is that there will only be one of these per user request.  A"},{"line_number":1005,"context_line":"    lookup by (instance_uuid, request_id) should always return a single result."},{"line_number":1006,"context_line":"    \"\"\""},{"line_number":1007,"context_line":"    __tablename__ \u003d \u0027instance_actions\u0027"},{"line_number":1008,"context_line":"    id \u003d Column(Integer, primary_key\u003dTrue, nullable\u003dFalse, autoincrement\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAM3%2F%2FPP8%3D","line":1005,"updated":"2013-01-31 21:17:59.000000000","message":"Shouldn\u0027t request_id be unique enough?","commit_id":"a56144969c6e1177bfaf542b764e92cbecc6103b"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"d3f60b1ab9155b61a817b82233f5ffc41d620b14","unresolved":false,"context_lines":[{"line_number":1002,"context_line":"    \"\"\"Track client actions on an instance."},{"line_number":1003,"context_line":""},{"line_number":1004,"context_line":"    The intention is that there will only be one of these per user request.  A"},{"line_number":1005,"context_line":"    lookup by (instance_uuid, request_id) should always return a single result."},{"line_number":1006,"context_line":"    \"\"\""},{"line_number":1007,"context_line":"    __tablename__ \u003d \u0027instance_actions\u0027"},{"line_number":1008,"context_line":"    id \u003d Column(Integer, primary_key\u003dTrue, nullable\u003dFalse, autoincrement\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAM3%2F%2FPO0%3D","line":1005,"in_reply_to":"AAAAM3%2F%2FPP8%3D","updated":"2013-01-31 21:53:53.000000000","message":"Usually, yes.  But if the multiple_create extension is loaded it\u0027s possible to have multiple instance create actions with a single request id.","commit_id":"a56144969c6e1177bfaf542b764e92cbecc6103b"}],"nova/scheduler/manager.py":[{"author":{"_account_id":1528,"name":"Stanislaw Pitucha","email":"viraptor@gmail.com","username":"stanislaw-pitucha"},"change_message_id":"66fb732ff6abe0bb206bfdcc2067c650ac613c80","unresolved":false,"context_lines":[{"line_number":142,"context_line":"                self._set_vm_state_and_notify(\u0027run_instance\u0027,"},{"line_number":143,"context_line":"                                             {\u0027vm_state\u0027: vm_states.ERROR,"},{"line_number":144,"context_line":"                                              \u0027task_state\u0027: None},"},{"line_number":145,"context_line":"                                             context, ex, request_spec)"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"    def prep_resize(self, context, image, request_spec, filter_properties,"},{"line_number":148,"context_line":"                    instance, instance_type, reservations):"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAM3%2F%2FfxE%3D","line":145,"updated":"2013-01-18 14:00:38.000000000","message":"Look like a fragile pattern (remember all exception handlers, report all finishes, etc.). What do you think about a more complex implementation, but simpler usage? \"with\" block can handle exceptions and it could take an expanded list of uuid in the resource initializer. So something like \"__init__(self, context, event_name, *uuids)\". This makes the single usage simpler: \"with EventReporter(context, \"schedule\", *instance_uuids):\"\n\nThere\u0027s still the case of exception handlers which don\u0027t re-raise... but maybe we could have a method on the resource manager to handle that explicitly:\n\n    with EventReporter(context, event_name, *instance_uuids) as event_reporter:\n        try:\n            ...\n        except exception.NoValidHost as ex:\n            event_reporeter.all_failed(ex)\n            ...\n\nThere could be also a .one_failed(uuid, ex) if it\u0027s needed that doesn\u0027t report a success for that one uuid on exit (but does for the rest).","commit_id":"a7f1802a5dbbad5517ccfa218c8ebd4cde09566a"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"4abc09ce122fde5cc857bd5fdcb398227427a149","unresolved":false,"context_lines":[{"line_number":142,"context_line":"                self._set_vm_state_and_notify(\u0027run_instance\u0027,"},{"line_number":143,"context_line":"                                             {\u0027vm_state\u0027: vm_states.ERROR,"},{"line_number":144,"context_line":"                                              \u0027task_state\u0027: None},"},{"line_number":145,"context_line":"                                             context, ex, request_spec)"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"    def prep_resize(self, context, image, request_spec, filter_properties,"},{"line_number":148,"context_line":"                    instance, instance_type, reservations):"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAM3%2F%2Fd00%3D","line":145,"in_reply_to":"AAAAM3%2F%2FfxE%3D","updated":"2013-01-21 14:43:54.000000000","message":"I\u0027ll play around with this and see how it works out.  Thanks for the suggestion, I agree it could end making usage much easier.","commit_id":"a7f1802a5dbbad5517ccfa218c8ebd4cde09566a"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"56cb7ab0997d362e7128f418c91f2b6a26bf19bb","unresolved":false,"context_lines":[{"line_number":106,"context_line":"        instance_uuids \u003d request_spec[\u0027instance_uuids\u0027]"},{"line_number":107,"context_line":"        with compute_utils.EventReporter(context, \u0027schedule\u0027, *instance_uuids):"},{"line_number":108,"context_line":"            try:"},{"line_number":109,"context_line":"                self.driver.schedule_run_instance(context,"},{"line_number":110,"context_line":"                        request_spec, admin_password, injected_files,"},{"line_number":111,"context_line":"                        requested_networks, is_first_time, filter_properties)"},{"line_number":112,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAM3%2F%2FVNA%3D","line":109,"updated":"2013-01-28 18:39:31.000000000","message":"You lost the \u0027return\u0027 here.  Is that a problem?","commit_id":"99ac92bcc379e819b2f7baa6a55a0cbb1426d158"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"6f12a224fa7382cca702a899c378ffc44ed88bf4","unresolved":false,"context_lines":[{"line_number":106,"context_line":"        instance_uuids \u003d request_spec[\u0027instance_uuids\u0027]"},{"line_number":107,"context_line":"        with compute_utils.EventReporter(context, \u0027schedule\u0027, *instance_uuids):"},{"line_number":108,"context_line":"            try:"},{"line_number":109,"context_line":"                self.driver.schedule_run_instance(context,"},{"line_number":110,"context_line":"                        request_spec, admin_password, injected_files,"},{"line_number":111,"context_line":"                        requested_networks, is_first_time, filter_properties)"},{"line_number":112,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAM3%2F%2FVDc%3D","line":109,"in_reply_to":"AAAAM3%2F%2FVNA%3D","updated":"2013-01-28 20:07:58.000000000","message":"It\u0027s not a problem, since it ends up as a cast eventually.  I removed them because I originally had code after this line.  It was moved to the contextmanager so I\u0027ll add the returns back in to minimize changes.","commit_id":"99ac92bcc379e819b2f7baa6a55a0cbb1426d158"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"56cb7ab0997d362e7128f418c91f2b6a26bf19bb","unresolved":false,"context_lines":[{"line_number":141,"context_line":"                    \u0027instance_type\u0027: instance_type,"},{"line_number":142,"context_line":"                    \u0027reservations\u0027: reservations,"},{"line_number":143,"context_line":"                }"},{"line_number":144,"context_line":"                self.driver.schedule_prep_resize(**kwargs)"},{"line_number":145,"context_line":"            except exception.NoValidHost as ex:"},{"line_number":146,"context_line":"                self._set_vm_state_and_notify(\u0027prep_resize\u0027,"},{"line_number":147,"context_line":"                                             {\u0027vm_state\u0027: vm_states.ACTIVE,"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAM3%2F%2FVM8%3D","line":144,"updated":"2013-01-28 18:39:31.000000000","message":"Again, you lost the \u0027return\u0027 here…","commit_id":"99ac92bcc379e819b2f7baa6a55a0cbb1426d158"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"6f12a224fa7382cca702a899c378ffc44ed88bf4","unresolved":false,"context_lines":[{"line_number":141,"context_line":"                    \u0027instance_type\u0027: instance_type,"},{"line_number":142,"context_line":"                    \u0027reservations\u0027: reservations,"},{"line_number":143,"context_line":"                }"},{"line_number":144,"context_line":"                self.driver.schedule_prep_resize(**kwargs)"},{"line_number":145,"context_line":"            except exception.NoValidHost as ex:"},{"line_number":146,"context_line":"                self._set_vm_state_and_notify(\u0027prep_resize\u0027,"},{"line_number":147,"context_line":"                                             {\u0027vm_state\u0027: vm_states.ACTIVE,"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAM3%2F%2FVDY%3D","line":144,"in_reply_to":"AAAAM3%2F%2FVM8%3D","updated":"2013-01-28 20:07:58.000000000","message":"Same as above, but will re-add.","commit_id":"99ac92bcc379e819b2f7baa6a55a0cbb1426d158"}],"nova/tests/fake_instance_actions.py":[{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"56cb7ab0997d362e7128f418c91f2b6a26bf19bb","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# vim: tabstop\u003d4 shiftwidth\u003d4 softtabstop\u003d4"},{"line_number":2,"context_line":"#"},{"line_number":3,"context_line":"#    Copyright 2010 OpenStack LLC"},{"line_number":4,"context_line":"#"},{"line_number":5,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":6,"context_line":"#    not use this file except in compliance with the License. You may obtain"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAM3%2F%2FVM0%3D","line":3,"updated":"2013-01-28 18:39:31.000000000","message":"Copyright date is a little…dated :)","commit_id":"99ac92bcc379e819b2f7baa6a55a0cbb1426d158"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"6f12a224fa7382cca702a899c378ffc44ed88bf4","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# vim: tabstop\u003d4 shiftwidth\u003d4 softtabstop\u003d4"},{"line_number":2,"context_line":"#"},{"line_number":3,"context_line":"#    Copyright 2010 OpenStack LLC"},{"line_number":4,"context_line":"#"},{"line_number":5,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":6,"context_line":"#    not use this file except in compliance with the License. You may obtain"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAM3%2F%2FVFI%3D","line":3,"in_reply_to":"AAAAM3%2F%2FVM0%3D","updated":"2013-01-28 20:07:58.000000000","message":"Well played.  Fixed.","commit_id":"99ac92bcc379e819b2f7baa6a55a0cbb1426d158"}],"nova/utils.py":[{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"d1c313befff29e573906d6dd993aa158f498fd57","unresolved":false,"context_lines":[{"line_number":1329,"context_line":"    #NOTE(alaski) the implicit \u0027self\u0027 or \u0027cls\u0027 argument shows up in"},{"line_number":1330,"context_line":"    # argnames but not in args or kwargs.  Uses \u0027in\u0027 rather than \u0027\u003d\u003d\u0027 because"},{"line_number":1331,"context_line":"    # some tests use \u0027self2\u0027."},{"line_number":1332,"context_line":"    if \u0027self\u0027 in argnames[0] or \u0027cls\u0027 \u003d\u003d argnames[0]:"},{"line_number":1333,"context_line":"        # The function may not actually be a method or have im_self."},{"line_number":1334,"context_line":"        # Typically seen when it\u0027s stubbed with mox."},{"line_number":1335,"context_line":"        if inspect.ismethod(function) and hasattr(function, \u0027im_self\u0027):"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAM3%2F%2FNsM%3D","line":1332,"updated":"2013-02-01 21:59:09.000000000","message":"I would suggest just using inspect.ismethod() here.  I tend to use \u0027inst\u0027 instead of \u0027self2\u0027 in unit tests…","commit_id":"250230b32364b1e36ae6d62ec4bb8c3285c59401"}]}