)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"6421982342446b67bde5ba5c2cd1a5422d1f8d28","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Samuel Merritt \u003csam@swiftstack.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-12-21 16:15:36 -0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Let operators add watchers to object audit"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Swift operators may find it useful to operate on each object in their"},{"line_number":10,"context_line":"cluster in some way. This commit provides them a way to hook into the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"3a7e1126_6def511e","line":7,"updated":"2015-12-22 00:21:42.000000000","message":"s/operators/developers/g","commit_id":"3b1ce108e9c8b12d7669460e3c8b4f25ee79d956"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"48d65e4022f2be319f6255e9839d138079949a45","unresolved":false,"context_lines":[{"line_number":22,"context_line":"This commit makes the auditor locate, via entry points, the watchers"},{"line_number":23,"context_line":"named in its config file."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"A watcher is a class with at least these four methods:"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"   __init__(self, conf, logger)"},{"line_number":28,"context_line":"   start(self, audit_type)"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"fa69d971_68573826","line":25,"updated":"2016-01-06 22:37:42.000000000","message":"Currently this is the only documentation of the methods we expect watchers to implement, right?\n\nWhat are the possible values for audit_type? policy_index and object_metadata seem likely to be ints and dicts, respectively. Is there an easy way for developers to get the policy name? Whether it\u0027s EC or replicated? What keys should we expect to have available in object_metadata?\n\nMaybe we could provide a simple no-op watcher that developers could subclass, and include more info in the docstrings.","commit_id":"3b1ce108e9c8b12d7669460e3c8b4f25ee79d956"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"495576f78b394db57a60390891e95b1af3abd1cd","unresolved":false,"context_lines":[{"line_number":27,"context_line":"   __init__(self, conf, logger)"},{"line_number":28,"context_line":"   start(self, audit_type)"},{"line_number":29,"context_line":"   see_object(self, policy_index, object_metadata)"},{"line_number":30,"context_line":"   end(self)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"The auditor will call watcher.start(audit_type) at the start of an"},{"line_number":33,"context_line":"audit pass, watcher.see_object(policy_index, metadata) for each object"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"7a5de9d1_6a037377","line":30,"updated":"2016-02-02 17:51:25.000000000","message":"Suggest adding *args, **kwargs to the expected method signatures to allow for future params to be added without breaking watchers.\n\nAlso, as mentioned in another review, I\u0027d like to have the audit_location passed. I haven;t tried too hard but I don\u0027t think the metadata and policy is sufficient to let me find construct a diskfile for the object - e.g. if I wanted to change some metadata, like a crypto re-keying process for example...","commit_id":"2adc20b68047cd33abe82f58f6eb8aaf4591cd6e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c62e5a0f2195787a5df4c5dd808eb0fdd2dd9095","unresolved":false,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"   start(self, audit_type, *args, **kwargs)"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"   see_object(self, sequence_number, policy_index, object_metadata,"},{"line_number":32,"context_line":"              partition, *args, **kwargs)"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"   end(self, *args, **kwargs)"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"9aed3d3a_eb781cfb","line":31,"range":{"start_line":31,"start_character":37,"end_line":31,"end_character":49},"updated":"2016-02-25 20:39:04.000000000","message":"The kwargs now must be named \"storage_policy_index\" and \"metadata\".","commit_id":"d621e09b6f495aeff6820a56d19cd843c6af8784"}],"etc/object-server.conf-sample":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"48d65e4022f2be319f6255e9839d138079949a45","unresolved":false,"context_lines":[{"line_number":300,"context_line":"# points and report the result after a full scan."},{"line_number":301,"context_line":"# object_size_stats \u003d"},{"line_number":302,"context_line":""},{"line_number":303,"context_line":"# A comma-separated list of watcher entry points. This lets operators"},{"line_number":304,"context_line":"# programmatically see audited objects."},{"line_number":305,"context_line":"# watchers \u003d"},{"line_number":306,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":5,"id":"fa69d971_c5e5ed08","line":303,"range":{"start_line":303,"start_character":28,"end_line":303,"end_character":48},"updated":"2016-01-06 22:37:42.000000000","message":"It seems like we should call out that these must be \"swift.object_audit_watcher\" entry points.","commit_id":"3b1ce108e9c8b12d7669460e3c8b4f25ee79d956"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"48d65e4022f2be319f6255e9839d138079949a45","unresolved":false,"context_lines":[{"line_number":302,"context_line":""},{"line_number":303,"context_line":"# A comma-separated list of watcher entry points. This lets operators"},{"line_number":304,"context_line":"# programmatically see audited objects."},{"line_number":305,"context_line":"# watchers \u003d"},{"line_number":306,"context_line":""},{"line_number":307,"context_line":"# Note: Put it at the beginning of the pipleline to profile all middleware. But"},{"line_number":308,"context_line":"# it is safer to put this after healthcheck."}],"source_content_type":"application/octet-stream","patch_set":5,"id":"fa69d971_28e060cc","line":305,"updated":"2016-01-06 22:37:42.000000000","message":"Should we maybe also mention that each watcher spawns a new Python process? A comma-separated list in a config file kinda makes this feel like a lightweight thing, and doesn\u0027t scream \"hope you have the processors and memory to deal with this\" quite like I\u0027d expect.","commit_id":"3b1ce108e9c8b12d7669460e3c8b4f25ee79d956"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"495576f78b394db57a60390891e95b1af3abd1cd","unresolved":false,"context_lines":[{"line_number":301,"context_line":"# object_size_stats \u003d"},{"line_number":302,"context_line":""},{"line_number":303,"context_line":"# A comma-separated list of watcher entry points. This lets operators"},{"line_number":304,"context_line":"# programmatically see audited objects."},{"line_number":305,"context_line":"# watchers \u003d"},{"line_number":306,"context_line":""},{"line_number":307,"context_line":"# Note: Put it at the beginning of the pipleline to profile all middleware. But"}],"source_content_type":"application/octet-stream","patch_set":7,"id":"7a5de9d1_2094bde6","line":304,"updated":"2016-02-02 17:51:25.000000000","message":"Would be helpful to also document here the entry-point group name where they are expected to be registered","commit_id":"2adc20b68047cd33abe82f58f6eb8aaf4591cd6e"}],"swift/obj/auditor.py":[{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"8502578fc2cffebfa5488ea28a68f0043494a8a8","unresolved":false,"context_lines":[{"line_number":219,"context_line":"                    self.record_stats(obj_size)"},{"line_number":220,"context_line":"                for watcher in self.watchers:"},{"line_number":221,"context_line":"                    try:"},{"line_number":222,"context_line":"                        watcher.see_object(metadata.copy())"},{"line_number":223,"context_line":"                    except:  # noqa"},{"line_number":224,"context_line":"                        self.logger.exception("},{"line_number":225,"context_line":"                            \"Watcher %r crashed in see_object() for %s\","}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_9be11dd3","line":222,"updated":"2015-08-19 11:56:29.000000000","message":"I think it would be useful to call see_object() with the location of the object as well","commit_id":"898e93955f43794f8c7784a9687061092da3e3d5"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"48d65e4022f2be319f6255e9839d138079949a45","unresolved":false,"context_lines":[{"line_number":260,"context_line":"                self.watcher_classes.append(available_watchers[name].load())"},{"line_number":261,"context_line":"                self.logger.debug(\"Loaded entry point \u0027%s\u0027\", name)"},{"line_number":262,"context_line":"            else:"},{"line_number":263,"context_line":"                self.logger.warn("},{"line_number":264,"context_line":"                    \"Unable to locate entry point for watcher \u0027%s\u0027; skipping\","},{"line_number":265,"context_line":"                    name)"},{"line_number":266,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"fa69d971_65f1a194","line":263,"range":{"start_line":263,"start_character":28,"end_line":263,"end_character":32},"updated":"2016-01-06 22:37:42.000000000","message":"From https://review.openstack.org/#/c/208222/ I think we want warning(), not warn().","commit_id":"3b1ce108e9c8b12d7669460e3c8b4f25ee79d956"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"48d65e4022f2be319f6255e9839d138079949a45","unresolved":false,"context_lines":[{"line_number":454,"context_line":"    def _put(self, item):"},{"line_number":455,"context_line":"        try:"},{"line_number":456,"context_line":"            self._queue.put_nowait(item)"},{"line_number":457,"context_line":"        except multiprocessing.Queue.Full:"},{"line_number":458,"context_line":"            pass"},{"line_number":459,"context_line":""},{"line_number":460,"context_line":"    def _child_process(self):"},{"line_number":461,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa69d971_08df9c3a","line":458,"range":{"start_line":457,"start_character":8,"end_line":458,"end_character":16},"updated":"2016-01-06 22:37:42.000000000","message":"Do we have a feeling for how long a watcher has to process an item?","commit_id":"3b1ce108e9c8b12d7669460e3c8b4f25ee79d956"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"48d65e4022f2be319f6255e9839d138079949a45","unresolved":false,"context_lines":[{"line_number":461,"context_line":"        try:"},{"line_number":462,"context_line":"            watcher \u003d self.watcher_class(self.conf, self.logger)"},{"line_number":463,"context_line":"        except:  # noqa"},{"line_number":464,"context_line":"            self.logger.exception(\"Error instantiating watcher %r\","},{"line_number":465,"context_line":"                                  self.watcher_class)"},{"line_number":466,"context_line":"            return"},{"line_number":467,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"fa69d971_254de999","line":464,"range":{"start_line":464,"start_character":24,"end_line":464,"end_character":33},"updated":"2016-01-06 22:37:42.000000000","message":"Just to confirm: .exception() will include the traceback and exception name automatically, right?","commit_id":"3b1ce108e9c8b12d7669460e3c8b4f25ee79d956"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"48d65e4022f2be319f6255e9839d138079949a45","unresolved":false,"context_lines":[{"line_number":489,"context_line":"        except:  # noqa"},{"line_number":490,"context_line":"            # Yes, really catch everything; this is user-provided code, so"},{"line_number":491,"context_line":"            # there\u0027s no way of knowing *anything* about its quality."},{"line_number":492,"context_line":"            pass"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa69d971_a5b31997","line":492,"range":{"start_line":492,"start_character":12,"end_line":492,"end_character":16},"updated":"2016-01-06 22:37:42.000000000","message":"Should we at least log the exception? Otherwise, I think it\u0027ll be pretty hard to debug watchers...","commit_id":"3b1ce108e9c8b12d7669460e3c8b4f25ee79d956"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"495576f78b394db57a60390891e95b1af3abd1cd","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        self.stats_buckets \u003d dict("},{"line_number":67,"context_line":"            [(s, 0) for s in self.stats_sizes + [\u0027OVER\u0027]])"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"        self.watchers \u003d [WatcherWrapper(watcher_class, conf, logger)"},{"line_number":70,"context_line":"                         for watcher_class in watcher_classes]"},{"line_number":71,"context_line":"        logger.debug(\"%d audit watcher(s) loaded\", len(self.watchers))"},{"line_number":72,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"7a5de9d1_9b540e22","line":69,"updated":"2016-02-02 17:51:25.000000000","message":"Passing the watcher the conf dict does not help the watcher with options that are not in config where default values are used - I\u0027d prefer to see the watcher passed a dict of the actual auditor parameters (see also comment below about needing to pass a conf *new* dict to watcher).","commit_id":"2adc20b68047cd33abe82f58f6eb8aaf4591cd6e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"fa11a6eb599d9fe58970b5903412b0d952e4c98e","unresolved":false,"context_lines":[{"line_number":219,"context_line":"                    # can be rather large (i.e. tens of megabytes), so we"},{"line_number":220,"context_line":"                    # pass just the index so we can keep the messages to our"},{"line_number":221,"context_line":"                    # child process small."},{"line_number":222,"context_line":"                    watcher.see_object(metadata, location.policy.idx)"},{"line_number":223,"context_line":"                if self.zero_byte_only_at_fps and obj_size:"},{"line_number":224,"context_line":"                    self.passes +\u003d 1"},{"line_number":225,"context_line":"                    return"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a68dd71_c7577a58","line":222,"range":{"start_line":222,"start_character":20,"end_line":222,"end_character":69},"updated":"2016-01-23 00:31:55.000000000","message":"Down below (and in the commit message), we have a signature of\n\n    def see_object(self, policy_index, meta)\n\nThis one\u0027s backwards.","commit_id":"2adc20b68047cd33abe82f58f6eb8aaf4591cd6e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"fa11a6eb599d9fe58970b5903412b0d952e4c98e","unresolved":false,"context_lines":[{"line_number":270,"context_line":"                self.watcher_classes.append(available_watchers[name].load())"},{"line_number":271,"context_line":"                self.logger.debug(\"Loaded entry point \u0027%s\u0027\", name)"},{"line_number":272,"context_line":"            else:"},{"line_number":273,"context_line":"                self.logger.warn("},{"line_number":274,"context_line":"                    \"Unable to locate entry point for watcher \u0027%s\u0027; skipping\","},{"line_number":275,"context_line":"                    name)"},{"line_number":276,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"9a68dd71_67156ee3","line":273,"range":{"start_line":273,"start_character":16,"end_line":273,"end_character":32},"updated":"2016-01-23 00:31:55.000000000","message":"Huh, it was worse than I previously thought:\n\n  AttributeError: \u0027LogAdapter\u0027 object has no attribute \u0027warn\u0027","commit_id":"2adc20b68047cd33abe82f58f6eb8aaf4591cd6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"495576f78b394db57a60390891e95b1af3abd1cd","unresolved":false,"context_lines":[{"line_number":410,"context_line":"    or use of os._exit()."},{"line_number":411,"context_line":""},{"line_number":412,"context_line":"    We also let the auditor continue auditing at its full speed without"},{"line_number":413,"context_line":"    pausing after every object to call some user-supplied code."},{"line_number":414,"context_line":"    \"\"\""},{"line_number":415,"context_line":""},{"line_number":416,"context_line":"    SUBPROCESS_TERM_TIMEOUT \u003d 0.5  # seconds"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a5de9d1_8f47e44a","line":413,"updated":"2016-02-02 17:51:25.000000000","message":"OK, good design. But it means that the auditor-\u003ewatcher communication is unreliable (since its using Queue.put_nowait) so a watcher can never be sure if it has missed objects during a cycle (start-\u003eend). I realise that often/usually the device contents are changing dynamically as the auditor runs, so there is no concept of \"all objects have been visited\", but there may well be uses cases (e.g. watching for objects in a retired policy or account, watching for objects older than T) where the watcher would really like some confidence that is has seen all objects that it may be looking for.\n\nThat could be helped by passing watchers the \u0027total_files_processed counter either as a param to see_object and/or as a param to end().\n\nAlso, maybe its worth trying a little harder to get the start and end messages onto the queue? Or, assign each AuditWorker a unique id and pass it with see_object calls...so the watcher has some clue as to where the auditor has got to wrt objects processed, cycles repeated...\n\nOK, so i don\u0027t know the answer, but hopefully you can understand my concern.","commit_id":"2adc20b68047cd33abe82f58f6eb8aaf4591cd6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"495576f78b394db57a60390891e95b1af3abd1cd","unresolved":false,"context_lines":[{"line_number":416,"context_line":"    SUBPROCESS_TERM_TIMEOUT \u003d 0.5  # seconds"},{"line_number":417,"context_line":"    SUBPROCESS_KILL_TIMEOUT \u003d 1.0  # seconds"},{"line_number":418,"context_line":""},{"line_number":419,"context_line":"    def __init__(self, watcher_class, conf, logger, max_queue_size\u003d256):"},{"line_number":420,"context_line":"        self.watcher_class \u003d watcher_class"},{"line_number":421,"context_line":"        self.conf \u003d conf"},{"line_number":422,"context_line":"        self.logger \u003d logger"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a5de9d1_f1a3e1fe","line":419,"updated":"2016-02-02 17:51:25.000000000","message":"max_queue_size is never passed with an arg - was the intention that this might be a configurable size?","commit_id":"2adc20b68047cd33abe82f58f6eb8aaf4591cd6e"},{"author":{"_account_id":18978,"name":"Catherine Northcott","email":"catherine@northcott.nz","username":"Zyric"},"change_message_id":"029bd1644d7f492a3e9d15bdf3e147675ced8af9","unresolved":false,"context_lines":[{"line_number":429,"context_line":"    def start(self, audit_type):"},{"line_number":430,"context_line":"        self._put([\u0027start\u0027, audit_type])"},{"line_number":431,"context_line":""},{"line_number":432,"context_line":"    def see_object(self, policy_index, meta):"},{"line_number":433,"context_line":"        self._put([\u0027see_object\u0027, policy_index, meta])"},{"line_number":434,"context_line":""},{"line_number":435,"context_line":"    def end(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"dae33548_6e28e4e2","line":432,"range":{"start_line":432,"start_character":8,"end_line":432,"end_character":18},"updated":"2016-02-18 00:04:21.000000000","message":"If you change this to a more abstract name like \"see_target\" and add support for *args/**kargs this watcher wrapper becomes usable for account and container watchers too.","commit_id":"2adc20b68047cd33abe82f58f6eb8aaf4591cd6e"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"5488020a9bd2fde10e98780ac28518b08d15b2b1","unresolved":false,"context_lines":[{"line_number":429,"context_line":"    def start(self, audit_type):"},{"line_number":430,"context_line":"        self._put([\u0027start\u0027, audit_type])"},{"line_number":431,"context_line":""},{"line_number":432,"context_line":"    def see_object(self, policy_index, meta):"},{"line_number":433,"context_line":"        self._put([\u0027see_object\u0027, policy_index, meta])"},{"line_number":434,"context_line":""},{"line_number":435,"context_line":"    def end(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"3fa7e38b_c73480e3","line":432,"range":{"start_line":432,"start_character":8,"end_line":432,"end_character":18},"in_reply_to":"dae33548_6e28e4e2","updated":"2020-01-11 16:05:59.000000000","message":"The thought crossed my mind, but do you think the protocol should be the same across all watchers? Notice that accounts do not have policies. One can always supply None, but it only means you\u0027re creating an artificial polymorphism where it\u0027s not existing naturally.\n\nBut on the other hand accounts and containers are in a sense objects, so even if we extend this to all types of objects, \"see_object\" continues, to apply.\n\nI think we need more plugin implementations first, and bikeshed this names of methods when the bulk is done.","commit_id":"2adc20b68047cd33abe82f58f6eb8aaf4591cd6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"495576f78b394db57a60390891e95b1af3abd1cd","unresolved":false,"context_lines":[{"line_number":465,"context_line":"    def _put(self, item):"},{"line_number":466,"context_line":"        try:"},{"line_number":467,"context_line":"            self._queue.put_nowait(item)"},{"line_number":468,"context_line":"        except multiprocessing.Queue.Full:"},{"line_number":469,"context_line":"            pass"},{"line_number":470,"context_line":""},{"line_number":471,"context_line":"    def _child_process(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a5de9d1_807d71b3","line":468,"updated":"2016-02-02 17:51:25.000000000","message":"Full is not an attribute of multiprocessing.Queue, look like it needs to be imported from Queue package","commit_id":"2adc20b68047cd33abe82f58f6eb8aaf4591cd6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"495576f78b394db57a60390891e95b1af3abd1cd","unresolved":false,"context_lines":[{"line_number":470,"context_line":""},{"line_number":471,"context_line":"    def _child_process(self):"},{"line_number":472,"context_line":"        try:"},{"line_number":473,"context_line":"            watcher \u003d self.watcher_class(self.conf, self.logger)"},{"line_number":474,"context_line":"        except:  # noqa"},{"line_number":475,"context_line":"            self.logger.exception(\"Error instantiating watcher %r\","},{"line_number":476,"context_line":"                                  self.watcher_class)"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a5de9d1_a98503ae","line":473,"updated":"2016-02-02 17:51:25.000000000","message":"It would be prudent to pass the watcher a *copy* of the conf so that the auditor is isolated from any changes the watcher class might make to the conf dict.\n\nI guess the same concern applies to the logger, but I\u0027m not sure quite what to suggest there.","commit_id":"2adc20b68047cd33abe82f58f6eb8aaf4591cd6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"495576f78b394db57a60390891e95b1af3abd1cd","unresolved":false,"context_lines":[{"line_number":474,"context_line":"        except:  # noqa"},{"line_number":475,"context_line":"            self.logger.exception(\"Error instantiating watcher %r\","},{"line_number":476,"context_line":"                                  self.watcher_class)"},{"line_number":477,"context_line":"            return"},{"line_number":478,"context_line":""},{"line_number":479,"context_line":"        while True:"},{"line_number":480,"context_line":"            item \u003d self._queue.get()  # block forever"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a5de9d1_aa196b24","line":477,"updated":"2016-02-02 17:51:25.000000000","message":"could set a \u0027running\u0027 flag here and not bother filling the queue if running\u003d\u003dFalse because the watcher failed to be instantiated.","commit_id":"2adc20b68047cd33abe82f58f6eb8aaf4591cd6e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c0d94c0cbe0c7d6a6018babdd11c73cf882e5fa6","unresolved":false,"context_lines":[{"line_number":488,"context_line":"        try:"},{"line_number":489,"context_line":"            if item[0] \u003d\u003d \u0027start\u0027:"},{"line_number":490,"context_line":"                audit_type \u003d item[1]"},{"line_number":491,"context_line":"                watcher.start(audit_type)"},{"line_number":492,"context_line":"            elif item[0] \u003d\u003d \u0027see_object\u0027:"},{"line_number":493,"context_line":"                policy_index \u003d item[1]"},{"line_number":494,"context_line":"                obj_meta \u003d item[2]"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a5de9d1_94005bac","line":491,"updated":"2016-01-27 17:13:43.000000000","message":"Do we expect that most watchers will want to watch both audit types?\n\nIf not, should we provide a way to have the watcher tell us to skip it as part of start()? exit() and sys.exit() will just get ignored in the except.\n\nI guess there\u0027s still os._exit() but that seems heavy-handed. I suppose we\u0027ve already designed this to be extremely isolated... so maybe it doesn\u0027t matter. At the very least, though, we should document that as the expected (only?) method of early termination.","commit_id":"2adc20b68047cd33abe82f58f6eb8aaf4591cd6e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"fa11a6eb599d9fe58970b5903412b0d952e4c98e","unresolved":false,"context_lines":[{"line_number":500,"context_line":"        except:  # noqa"},{"line_number":501,"context_line":"            # Yes, really catch everything; this is user-provided code, so"},{"line_number":502,"context_line":"            # there\u0027s no way of knowing *anything* about its quality."},{"line_number":503,"context_line":"            pass"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a68dd71_87cc226b","line":503,"range":{"start_line":503,"start_character":12,"end_line":503,"end_character":16},"updated":"2016-01-23 00:31:55.000000000","message":"I still think it\u0027s worth logging the exception rather than relying on developers to properly wrap start/see_object/end in try...except clauses on their own.","commit_id":"2adc20b68047cd33abe82f58f6eb8aaf4591cd6e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c62e5a0f2195787a5df4c5dd808eb0fdd2dd9095","unresolved":false,"context_lines":[{"line_number":272,"context_line":"                self.watcher_classes.append(available_watchers[name].load())"},{"line_number":273,"context_line":"                self.logger.debug(\"Loaded entry point \u0027%s\u0027\", name)"},{"line_number":274,"context_line":"            else:"},{"line_number":275,"context_line":"                self.logger.warn("},{"line_number":276,"context_line":"                    \"Unable to locate entry point for watcher \u0027%s\u0027; skipping\","},{"line_number":277,"context_line":"                    name)"},{"line_number":278,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9aed3d3a_2bd164ce","line":275,"range":{"start_line":275,"start_character":28,"end_line":275,"end_character":32},"updated":"2016-02-25 20:39:04.000000000","message":"While no longer strictly *necessary* (at least on tip-of-master), it\u0027d probably still be good to have this use self.logger.warning()","commit_id":"d621e09b6f495aeff6820a56d19cd843c6af8784"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c62e5a0f2195787a5df4c5dd808eb0fdd2dd9095","unresolved":false,"context_lines":[{"line_number":445,"context_line":"        Send the \u0027end\u0027 message to the watcher and also shut down the child"},{"line_number":446,"context_line":"        process."},{"line_number":447,"context_line":"        \"\"\""},{"line_number":448,"context_line":"        self._put([\u0027end\u0027, {}])"},{"line_number":449,"context_line":"        self._put(None)"},{"line_number":450,"context_line":"        sentinel_at \u003d time.time()"},{"line_number":451,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9aed3d3a_1c402456","line":448,"range":{"start_line":448,"start_character":26,"end_line":448,"end_character":28},"updated":"2016-02-25 20:39:04.000000000","message":"We never use this dict. Was the intention to move everything in _feed_item_to_watcher() to using **kwargs like is done in the see_object case? If so, we should update start() as well.","commit_id":"d621e09b6f495aeff6820a56d19cd843c6af8784"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b2a731918e3c7a108b80c07f5d53230b8110a340","unresolved":false,"context_lines":[{"line_number":445,"context_line":"        Send the \u0027end\u0027 message to the watcher and also shut down the child"},{"line_number":446,"context_line":"        process."},{"line_number":447,"context_line":"        \"\"\""},{"line_number":448,"context_line":"        self._put([\u0027end\u0027, {}])"},{"line_number":449,"context_line":"        self._put(None)"},{"line_number":450,"context_line":"        sentinel_at \u003d time.time()"},{"line_number":451,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"1f769fc5_6040df36","line":448,"range":{"start_line":448,"start_character":26,"end_line":448,"end_character":28},"in_reply_to":"9aed3d3a_1c402456","updated":"2019-01-03 23:43:55.000000000","message":"Done","commit_id":"d621e09b6f495aeff6820a56d19cd843c6af8784"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c62e5a0f2195787a5df4c5dd808eb0fdd2dd9095","unresolved":false,"context_lines":[{"line_number":480,"context_line":"        # This method is what runs in the subprocess. The subprocess exits"},{"line_number":481,"context_line":"        # when this method returns."},{"line_number":482,"context_line":"        try:"},{"line_number":483,"context_line":"            watcher \u003d self.watcher_class(self.conf, self.logger)"},{"line_number":484,"context_line":"        except:  # noqa"},{"line_number":485,"context_line":"            self.logger.exception(\"Error instantiating watcher %r\","},{"line_number":486,"context_line":"                                  self.watcher_class)"}],"source_content_type":"text/x-python","patch_set":8,"id":"9aed3d3a_77a34b9f","line":483,"range":{"start_line":483,"start_character":41,"end_line":483,"end_character":50},"updated":"2016-02-25 20:39:04.000000000","message":"Still want to isolate this, per Alistair\u0027s suggestion.","commit_id":"d621e09b6f495aeff6820a56d19cd843c6af8784"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c62e5a0f2195787a5df4c5dd808eb0fdd2dd9095","unresolved":false,"context_lines":[{"line_number":498,"context_line":"        try:"},{"line_number":499,"context_line":"            if item[0] \u003d\u003d \u0027start\u0027:"},{"line_number":500,"context_line":"                audit_type \u003d item[1]"},{"line_number":501,"context_line":"                watcher.start(audit_type\u003daudit_type)"},{"line_number":502,"context_line":"            elif item[0] \u003d\u003d \u0027see_object\u0027:"},{"line_number":503,"context_line":"                kwargs \u003d item[1]"},{"line_number":504,"context_line":"                watcher.see_object(**kwargs)"}],"source_content_type":"text/x-python","patch_set":8,"id":"9aed3d3a_b755d3c7","line":501,"updated":"2016-02-25 20:39:04.000000000","message":"I\u0027d still like to see a way (other than os._exit()) for a watcher to signal that it doesn\u0027t care about this audit type. Alistair\u0027s \u0027running\u0027 flag idea seems like it may be useful; then we could just say\n\n self._running \u003d bool(watcher.start(audit_type\u003daudit_type))\n\nhere and change the loop above to\n\n while self._running:\n\nAlternatively, we could make our sentinel value a little more explicit and have something like\n\n for item in iter(self._queue.get, None):\n\nand separately break if not running.","commit_id":"d621e09b6f495aeff6820a56d19cd843c6af8784"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b2a731918e3c7a108b80c07f5d53230b8110a340","unresolved":false,"context_lines":[{"line_number":560,"context_line":"                                  self.watcher_class)"},{"line_number":561,"context_line":"            return"},{"line_number":562,"context_line":""},{"line_number":563,"context_line":"        while True:"},{"line_number":564,"context_line":"            item \u003d self._queue.get()  # block forever"},{"line_number":565,"context_line":"            if item is None:"},{"line_number":566,"context_line":"                # sentinel value meaning \"exit; work\u0027s complete\""}],"source_content_type":"text/x-python","patch_set":10,"id":"1f769fc5_2c366305","line":563,"updated":"2019-01-03 23:43:55.000000000","message":"Maybe worth doing something like\n\n for item in iter(self._queue.get, None):","commit_id":"716d9c17705cb68597d0d28014686354e26ba584"},{"author":{"_account_id":13852,"name":"Romain LE DISEZ","email":"romain.le-disez@corp.ovh.com","username":"rledisez"},"change_message_id":"7ff56ccd4b88404cc031ff2bf4f79767469a68b6","unresolved":false,"context_lines":[{"line_number":328,"context_line":"        watcher_names \u003d list_from_csv(conf.get(\u0027watchers\u0027, \u0027\u0027))"},{"line_number":329,"context_line":"        available_watchers \u003d {"},{"line_number":330,"context_line":"            ep.name: ep"},{"line_number":331,"context_line":"            for ep in pkg_resources.iter_entry_points("},{"line_number":332,"context_line":"                \"swift.object_audit_watcher\")}"},{"line_number":333,"context_line":""},{"line_number":334,"context_line":"        self.watcher_classes \u003d []"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_1fc8aaa3","line":331,"updated":"2019-02-01 15:03:10.000000000","message":"It should probably use common.utils.load_pkg_resource()","commit_id":"33d91e413a35bdae14b5fa1cfcd880225fb5b704"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ef35d0b2c59e281a7a16fb317d69e626c5448bb3","unresolved":false,"context_lines":[{"line_number":328,"context_line":"        watcher_names \u003d list_from_csv(conf.get(\u0027watchers\u0027, \u0027\u0027))"},{"line_number":329,"context_line":"        available_watchers \u003d {"},{"line_number":330,"context_line":"            ep.name: ep"},{"line_number":331,"context_line":"            for ep in pkg_resources.iter_entry_points("},{"line_number":332,"context_line":"                \"swift.object_audit_watcher\")}"},{"line_number":333,"context_line":""},{"line_number":334,"context_line":"        self.watcher_classes \u003d []"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_f52707c6","line":331,"in_reply_to":"9fdfeff1_1fc8aaa3","updated":"2019-02-01 17:31:45.000000000","message":"It\u0027ll change the config a bit, from\n\n watchers \u003d some_watcher\n\nto\n\n watchers \u003d some_package#some_watcher\n\nbut ultimately, I think that namespacing is probably a good thing. Will fix.","commit_id":"33d91e413a35bdae14b5fa1cfcd880225fb5b704"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c9a5a82d4a87fa5f345e806311017f50749c921b","unresolved":false,"context_lines":[{"line_number":328,"context_line":"        watcher_names \u003d list_from_csv(conf.get(\u0027watchers\u0027, \u0027\u0027))"},{"line_number":329,"context_line":"        available_watchers \u003d {"},{"line_number":330,"context_line":"            ep.name: ep"},{"line_number":331,"context_line":"            for ep in pkg_resources.iter_entry_points("},{"line_number":332,"context_line":"                \"swift.object_audit_watcher\")}"},{"line_number":333,"context_line":""},{"line_number":334,"context_line":"        self.watcher_classes \u003d []"}],"source_content_type":"text/x-python","patch_set":12,"id":"3fa7e38b_0d516062","line":331,"in_reply_to":"9fdfeff1_f52707c6","updated":"2019-11-05 09:39:02.000000000","message":"Done","commit_id":"33d91e413a35bdae14b5fa1cfcd880225fb5b704"},{"author":{"_account_id":13852,"name":"Romain LE DISEZ","email":"romain.le-disez@corp.ovh.com","username":"rledisez"},"change_message_id":"7ff56ccd4b88404cc031ff2bf4f79767469a68b6","unresolved":false,"context_lines":[{"line_number":333,"context_line":""},{"line_number":334,"context_line":"        self.watcher_classes \u003d []"},{"line_number":335,"context_line":"        for name in watcher_names:"},{"line_number":336,"context_line":"            if name in available_watchers:"},{"line_number":337,"context_line":"                self.watcher_classes.append(available_watchers[name].load())"},{"line_number":338,"context_line":"                self.logger.debug(\"Loaded entry point \u0027%s\u0027\", name)"},{"line_number":339,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_1f4d0a3d","line":336,"updated":"2019-02-01 15:03:10.000000000","message":"I prefer the process to crash than to not do what the operator think it is doing. At least if the process is not running, the operator has a chance to see it in his monitoring","commit_id":"33d91e413a35bdae14b5fa1cfcd880225fb5b704"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ef35d0b2c59e281a7a16fb317d69e626c5448bb3","unresolved":false,"context_lines":[{"line_number":333,"context_line":""},{"line_number":334,"context_line":"        self.watcher_classes \u003d []"},{"line_number":335,"context_line":"        for name in watcher_names:"},{"line_number":336,"context_line":"            if name in available_watchers:"},{"line_number":337,"context_line":"                self.watcher_classes.append(available_watchers[name].load())"},{"line_number":338,"context_line":"                self.logger.debug(\"Loaded entry point \u0027%s\u0027\", name)"},{"line_number":339,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_b0018d07","line":336,"in_reply_to":"9fdfeff1_1f4d0a3d","updated":"2019-02-01 17:31:45.000000000","message":"Yeah, that\u0027s fair. Will fix. (In fact, I think we may get this for free by switching to load_pkg_resource()...)","commit_id":"33d91e413a35bdae14b5fa1cfcd880225fb5b704"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c9a5a82d4a87fa5f345e806311017f50749c921b","unresolved":false,"context_lines":[{"line_number":333,"context_line":""},{"line_number":334,"context_line":"        self.watcher_classes \u003d []"},{"line_number":335,"context_line":"        for name in watcher_names:"},{"line_number":336,"context_line":"            if name in available_watchers:"},{"line_number":337,"context_line":"                self.watcher_classes.append(available_watchers[name].load())"},{"line_number":338,"context_line":"                self.logger.debug(\"Loaded entry point \u0027%s\u0027\", name)"},{"line_number":339,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":12,"id":"3fa7e38b_2dcd3c2f","line":336,"in_reply_to":"9fdfeff1_b0018d07","updated":"2019-11-05 09:39:02.000000000","message":"Totally *did* fall out of the load_pkg_resource goodness.","commit_id":"33d91e413a35bdae14b5fa1cfcd880225fb5b704"},{"author":{"_account_id":13852,"name":"Romain LE DISEZ","email":"romain.le-disez@corp.ovh.com","username":"rledisez"},"change_message_id":"7ff56ccd4b88404cc031ff2bf4f79767469a68b6","unresolved":false,"context_lines":[{"line_number":492,"context_line":"    \"\"\""},{"line_number":493,"context_line":""},{"line_number":494,"context_line":"    SUBPROCESS_TERM_TIMEOUT \u003d 0.5  # seconds"},{"line_number":495,"context_line":"    SUBPROCESS_KILL_TIMEOUT \u003d 1.0  # seconds"},{"line_number":496,"context_line":""},{"line_number":497,"context_line":"    def __init__(self, watcher_class, conf, logger, max_queue_size\u003d256):"},{"line_number":498,"context_line":"        self.watcher_class \u003d watcher_class"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_5f97123b","line":495,"updated":"2019-02-01 15:03:10.000000000","message":"For something that will run for hours/days/weeks, only allowing 1 seconds of extra time seems rude, no? I would make it at least configurable.","commit_id":"33d91e413a35bdae14b5fa1cfcd880225fb5b704"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ef35d0b2c59e281a7a16fb317d69e626c5448bb3","unresolved":false,"context_lines":[{"line_number":492,"context_line":"    \"\"\""},{"line_number":493,"context_line":""},{"line_number":494,"context_line":"    SUBPROCESS_TERM_TIMEOUT \u003d 0.5  # seconds"},{"line_number":495,"context_line":"    SUBPROCESS_KILL_TIMEOUT \u003d 1.0  # seconds"},{"line_number":496,"context_line":""},{"line_number":497,"context_line":"    def __init__(self, watcher_class, conf, logger, max_queue_size\u003d256):"},{"line_number":498,"context_line":"        self.watcher_class \u003d watcher_class"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_9071e9aa","line":495,"in_reply_to":"9fdfeff1_5f97123b","updated":"2019-02-01 17:31:45.000000000","message":"1s between SIGTERM an SIGKILL actually seems not-too-crazy to me... but yeah, we should probably make the timeout between sending the sentinel and SIGTERM\n\n- longer -- the poor subprocess may have a couple hundred items still to process -- and\n- configurable.\n\nWhat would a good default be? 10s? 30s?","commit_id":"33d91e413a35bdae14b5fa1cfcd880225fb5b704"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c9a5a82d4a87fa5f345e806311017f50749c921b","unresolved":false,"context_lines":[{"line_number":492,"context_line":"    \"\"\""},{"line_number":493,"context_line":""},{"line_number":494,"context_line":"    SUBPROCESS_TERM_TIMEOUT \u003d 0.5  # seconds"},{"line_number":495,"context_line":"    SUBPROCESS_KILL_TIMEOUT \u003d 1.0  # seconds"},{"line_number":496,"context_line":""},{"line_number":497,"context_line":"    def __init__(self, watcher_class, conf, logger, max_queue_size\u003d256):"},{"line_number":498,"context_line":"        self.watcher_class \u003d watcher_class"}],"source_content_type":"text/x-python","patch_set":12,"id":"3fa7e38b_cddb48e7","line":495,"in_reply_to":"9fdfeff1_9071e9aa","updated":"2019-11-05 09:39:02.000000000","message":"Done\n\nNow configurable, as watcher_term_timeout and watcher_kill_timout.","commit_id":"33d91e413a35bdae14b5fa1cfcd880225fb5b704"}],"test/unit/obj/test_auditor.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"a781d5e4ea2cb4fd5a612af44f581a957d428a13","unresolved":false,"context_lines":[{"line_number":1671,"context_line":""},{"line_number":1672,"context_line":"        # TODO(jaegerandi): Remove return again."},{"line_number":1673,"context_line":"        # This test breaks Infra CI, disable for now"},{"line_number":1674,"context_line":"        return"},{"line_number":1675,"context_line":""},{"line_number":1676,"context_line":"        class WorkingWatcher(object):"},{"line_number":1677,"context_line":"            def __init__(self, conf, logger):"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f7c97a3_7042400f","line":1674,"updated":"2018-07-31 23:35:08.000000000","message":"We still need to get this part sorted out, I think :-/","commit_id":"716d9c17705cb68597d0d28014686354e26ba584"}]}