)]}'
{"doc/source/overview_container_sync.rst":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7a10ba36645654a9bc283d36870616a28d09361d","unresolved":false,"context_lines":[{"line_number":208,"context_line":"    That is, you need to run::"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"      swift-init container-sync once"},{"line_number":211,"context_line":"      swift-init container-sync once"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"Now expect to see objects copied from the first container to the second::"},{"line_number":214,"context_line":""}],"source_content_type":"text/x-rst","patch_set":26,"id":"5a710552_0bead90f","line":211,"updated":"2015-12-15 17:38:10.000000000","message":"I\u0027m not sure I understand this comment.\n\nIf the issue you are describing here is the delay between PUT-ting an object and its container being updated, then running container-sync twice is no guarantee that the container updates have occurred (it helps because more time has passed, but its not a guarantee). You would need to run the updater on all object replica nodes to be sure.\n\nAlso the comment doesn\u0027t state that you\u0027d need to locate the node that the sync\u0027d container db is on and run the command on that node.","commit_id":"b57a7a810d7fd635d7f2107d147ee0ab72e01216"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"399151b9656704a7e2ee6329e40bd896a2cc4b23","unresolved":false,"context_lines":[{"line_number":208,"context_line":"    That is, you need to run::"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"      swift-init container-sync once"},{"line_number":211,"context_line":"      swift-init container-sync once"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"Now expect to see objects copied from the first container to the second::"},{"line_number":214,"context_line":""}],"source_content_type":"text/x-rst","patch_set":26,"id":"5a710552_fd262a67","line":211,"in_reply_to":"5a710552_0bead90f","updated":"2015-12-16 06:44:37.000000000","message":"Here is a better explanation:\nIf you\u0027re an operator running SAIO and just testing, each time you configure a container for synchronization and place objects in the source container you will need to ensure that container-sync runs twice before attempting to retrieve objects from the target container. \nThis is because the container sync daemon processes each container in two phases: Assuming 3-way replication, the container sync daemon will first replicate only one third of the objects, leaving the other two thirds to the other two replicas. In the second phase the daemon does a second pass over the objects and attempts to replicate all of them.\nThus, in the first run only one of the source replica nodes attempts to replicate a given object, and due to the eventual consistency nature of Swift, that node\u0027s db might not have the new object yet. The second run assures that all nodes would attempt to replicate the object and one of them must have it. That is, you need to run::","commit_id":"b57a7a810d7fd635d7f2107d147ee0ab72e01216"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7a10ba36645654a9bc283d36870616a28d09361d","unresolved":false,"context_lines":[{"line_number":354,"context_line":"What\u0027s going on behind the scenes, in the cluster?"},{"line_number":355,"context_line":"--------------------------------------------------"},{"line_number":356,"context_line":""},{"line_number":357,"context_line":"Container ring devices have a container directory, where container databases"},{"line_number":358,"context_line":"reside. In addition to that directory, each container ring device also has a"},{"line_number":359,"context_line":"sync-containers directory. The sync-containers directory holds symlinks to"},{"line_number":360,"context_line":"container databases that were configured for container sync using the"}],"source_content_type":"text/x-rst","patch_set":26,"id":"5a710552_2bbeb5d4","line":357,"updated":"2015-12-15 17:38:10.000000000","message":"the directory is named \u0027containers\u0027 (plural)\n\nnit: double-back-tick formatting would be nice e.g. ``containers`` and so on wherever you specify a dir or filename. See overview_auth.rst for examples.","commit_id":"b57a7a810d7fd635d7f2107d147ee0ab72e01216"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"399151b9656704a7e2ee6329e40bd896a2cc4b23","unresolved":false,"context_lines":[{"line_number":354,"context_line":"What\u0027s going on behind the scenes, in the cluster?"},{"line_number":355,"context_line":"--------------------------------------------------"},{"line_number":356,"context_line":""},{"line_number":357,"context_line":"Container ring devices have a container directory, where container databases"},{"line_number":358,"context_line":"reside. In addition to that directory, each container ring device also has a"},{"line_number":359,"context_line":"sync-containers directory. The sync-containers directory holds symlinks to"},{"line_number":360,"context_line":"container databases that were configured for container sync using the"}],"source_content_type":"text/x-rst","patch_set":26,"id":"5a710552_dd1c8e15","line":357,"in_reply_to":"5a710552_2bbeb5d4","updated":"2015-12-16 06:44:37.000000000","message":"Tried to fix in a consistent manner across the file.","commit_id":"b57a7a810d7fd635d7f2107d147ee0ab72e01216"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7a10ba36645654a9bc283d36870616a28d09361d","unresolved":false,"context_lines":[{"line_number":375,"context_line":"scanning the local devices for container databases and checking for"},{"line_number":376,"context_line":"x-container-sync-to and x-container-sync-key metadata values. If they exist"},{"line_number":377,"context_line":"then a symlink to the container database is created in a sync-containers"},{"line_number":378,"context_line":"directory on the same device."},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"Similarly, when a container is being deleted or the container sync metadata"},{"line_number":381,"context_line":"keys are deleted, the container server and container-replicator would take"}],"source_content_type":"text/x-rst","patch_set":26,"id":"5a710552_cbb051e0","line":378,"updated":"2015-12-15 17:38:10.000000000","message":"nit: directory -\u003e sub-directory","commit_id":"b57a7a810d7fd635d7f2107d147ee0ab72e01216"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"399151b9656704a7e2ee6329e40bd896a2cc4b23","unresolved":false,"context_lines":[{"line_number":375,"context_line":"scanning the local devices for container databases and checking for"},{"line_number":376,"context_line":"x-container-sync-to and x-container-sync-key metadata values. If they exist"},{"line_number":377,"context_line":"then a symlink to the container database is created in a sync-containers"},{"line_number":378,"context_line":"directory on the same device."},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"Similarly, when a container is being deleted or the container sync metadata"},{"line_number":381,"context_line":"keys are deleted, the container server and container-replicator would take"}],"source_content_type":"text/x-rst","patch_set":26,"id":"5a710552_bd96624a","line":378,"in_reply_to":"5a710552_cbb051e0","updated":"2015-12-16 06:44:37.000000000","message":"fixed","commit_id":"b57a7a810d7fd635d7f2107d147ee0ab72e01216"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7a10ba36645654a9bc283d36870616a28d09361d","unresolved":false,"context_lines":[{"line_number":379,"context_line":""},{"line_number":380,"context_line":"Similarly, when a container is being deleted or the container sync metadata"},{"line_number":381,"context_line":"keys are deleted, the container server and container-replicator would take"},{"line_number":382,"context_line":"care of deleting the symlinks from the sync-containers directory."},{"line_number":383,"context_line":""},{"line_number":384,"context_line":".. note::"},{"line_number":385,"context_line":""}],"source_content_type":"text/x-rst","patch_set":26,"id":"5a710552_ab9f054f","line":382,"updated":"2015-12-15 17:38:10.000000000","message":"Right! but the server does not actually do that in this version :)","commit_id":"b57a7a810d7fd635d7f2107d147ee0ab72e01216"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"399151b9656704a7e2ee6329e40bd896a2cc4b23","unresolved":false,"context_lines":[{"line_number":379,"context_line":""},{"line_number":380,"context_line":"Similarly, when a container is being deleted or the container sync metadata"},{"line_number":381,"context_line":"keys are deleted, the container server and container-replicator would take"},{"line_number":382,"context_line":"care of deleting the symlinks from the sync-containers directory."},{"line_number":383,"context_line":""},{"line_number":384,"context_line":".. note::"},{"line_number":385,"context_line":""}],"source_content_type":"text/x-rst","patch_set":26,"id":"5a710552_9deda6f1","line":382,"in_reply_to":"5a710552_ab9f054f","updated":"2015-12-16 06:44:37.000000000","message":"yep, fixed and somewhat changed the description","commit_id":"b57a7a810d7fd635d7f2107d147ee0ab72e01216"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"8fc9ba0594a71c374901dc3602e032edb381a5fb","unresolved":false,"context_lines":[{"line_number":350,"context_line":"What\u0027s going on behind the scenes, in the cluster?"},{"line_number":351,"context_line":"--------------------------------------------------"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":"Container ring devices have a direcotry called ``containers``, where container"},{"line_number":354,"context_line":"databases reside. In addition to ``containers``, each container ring device"},{"line_number":355,"context_line":"also has a directory called ``sync-containers``. ``sync-containers`` holds"},{"line_number":356,"context_line":"symlinks to container databases that were configured for container sync using"}],"source_content_type":"text/x-rst","patch_set":29,"id":"fa69d971_9060f1a6","line":353,"range":{"start_line":353,"start_character":30,"end_line":353,"end_character":39},"updated":"2016-01-05 06:04:31.000000000","message":"s/direcotry/directory/","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"1894a6f77a2cee4c903ef90cbea78e7e8b593469","unresolved":false,"context_lines":[{"line_number":350,"context_line":"What\u0027s going on behind the scenes, in the cluster?"},{"line_number":351,"context_line":"--------------------------------------------------"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":"Container ring devices have a direcotry called ``containers``, where container"},{"line_number":354,"context_line":"databases reside. In addition to ``containers``, each container ring device"},{"line_number":355,"context_line":"also has a directory called ``sync-containers``. ``sync-containers`` holds"},{"line_number":356,"context_line":"symlinks to container databases that were configured for container sync using"}],"source_content_type":"text/x-rst","patch_set":29,"id":"fa69d971_361b21cc","line":353,"range":{"start_line":353,"start_character":30,"end_line":353,"end_character":39},"in_reply_to":"fa69d971_9060f1a6","updated":"2016-01-05 21:23:33.000000000","message":"good catch. thanks.","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"}],"swift/container/replicator.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a20455f7416ea5176823547eff8b278eeaa5e1d1","unresolved":false,"context_lines":[{"line_number":203,"context_line":"        \"\"\""},{"line_number":204,"context_line":"        Ensure that reconciler databases are only cleaned up at the end of the"},{"line_number":205,"context_line":"        replication run."},{"line_number":206,"context_line":"        \"\"\""},{"line_number":207,"context_line":"        if (self.reconciler_cleanups is not None and"},{"line_number":208,"context_line":"                broker.account \u003d\u003d MISPLACED_OBJECTS_ACCOUNT):"},{"line_number":209,"context_line":"            # this container shouldn\u0027t be here, make sure it\u0027s cleaned up"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa1b9901_f3bc246e","side":"PARENT","line":206,"updated":"2015-08-18 19:15:04.000000000","message":"I think this would be a decent place to hook in some optimistic cleanup of the sync links when a container db gets rebalanced off the node?  You might want to update some plumbing* so we don\u0027t have to call get_info again from inside here.\n\n* probably fix common.db_replicator to pass in (broker, info)","commit_id":"eb62f1b3796e615ed4c25ec97f43d470c127bfab"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a20455f7416ea5176823547eff8b278eeaa5e1d1","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        # We do this here rather then in the server upon PUT/POST as in the"},{"line_number":194,"context_line":"        # server we are not aware of the request vs.metadata timestamps"},{"line_number":195,"context_line":"        # At any rate we need to do this update from one of the background"},{"line_number":196,"context_line":"        # processes"},{"line_number":197,"context_line":"        self._update_sync_store(broker, info)"},{"line_number":198,"context_line":"        point \u003d broker.get_reconciler_sync()"},{"line_number":199,"context_line":"        if not broker.has_multiple_policies() and info[\u0027max_row\u0027] !\u003d point:"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_8f42ea3c","line":196,"updated":"2015-08-18 19:15:04.000000000","message":"this comment is probably better as a doc string on the _update_sync_store method?","commit_id":"aefd20882642662b4f7deeb19ddfa9617cc8c2ff"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a20455f7416ea5176823547eff8b278eeaa5e1d1","unresolved":false,"context_lines":[{"line_number":207,"context_line":"            broker.update_reconciler_sync(max_sync)"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"    def _update_sync_store(self, broker, info):"},{"line_number":210,"context_line":"        if not \u0027X-Container-Sync-To\u0027 in broker.metadata:"},{"line_number":211,"context_line":"            return"},{"line_number":212,"context_line":"        res \u003d True"},{"line_number":213,"context_line":"        sync_to \u003d broker.metadata[\u0027X-Container-Sync-To\u0027][0]"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_6f5c3e19","line":210,"updated":"2015-08-18 19:15:04.000000000","message":"Normally this would be written\n\n    if \u0027x-container-sync-to\u0027 not in broker.metadata:","commit_id":"aefd20882642662b4f7deeb19ddfa9617cc8c2ff"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a20455f7416ea5176823547eff8b278eeaa5e1d1","unresolved":false,"context_lines":[{"line_number":216,"context_line":"        else:"},{"line_number":217,"context_line":"            res \u003d self.sync_store.add_synced_container(broker)"},{"line_number":218,"context_line":"        if res is False:"},{"line_number":219,"context_line":"            self.logger.info(\u0027Failed to update sync_store %s\u0027 % broker.db_file)"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"    def delete_db(self, broker):"},{"line_number":222,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"fa1b9901_162ae67e","line":219,"updated":"2015-08-18 19:15:04.000000000","message":"This is only if there\u0027s some kind of unexpected OSError right?  it\u0027d be super helpful to have that traceback here.","commit_id":"aefd20882642662b4f7deeb19ddfa9617cc8c2ff"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"3adb96385ea732fefdca2b1763dd49d19885f8f2","unresolved":false,"context_lines":[{"line_number":224,"context_line":"                self.sync_store.add_synced_container(broker)"},{"line_number":225,"context_line":"        except Exception as e:"},{"line_number":226,"context_line":"            self.logger.info(\u0027Failed to update sync_store %s\u0027 % broker.db_file)"},{"line_number":227,"context_line":"            raise e"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"    def delete_db(self, broker):"},{"line_number":230,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":11,"id":"da20952f_46ba2573","line":227,"updated":"2015-09-08 19:00:17.000000000","message":"What happens with the exception at this point?  Is there any chance an exception here would interfere with the reconciler?  Can we just log the traceback here and eat it?","commit_id":"fcd483ad6bf1ab70d2e2ef26ad4b363863f73837"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"377478c9669b0ab3185759dfb02a536e80425103","unresolved":false,"context_lines":[{"line_number":207,"context_line":"            # replication"},{"line_number":208,"context_line":"            broker.update_reconciler_sync(max_sync)"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"    def _update_sync_store(self, broker, info):"},{"line_number":211,"context_line":"        \"\"\""},{"line_number":212,"context_line":"        Udpates the sync db post replication"},{"line_number":213,"context_line":"        We do this here rather then in the server upon PUT/POST as in the"}],"source_content_type":"text/x-python","patch_set":23,"id":"9a8ffd7b_d844dc1c","line":210,"updated":"2015-11-30 16:49:30.000000000","message":"I think it would be better separation of concerns if this was a method of ContainerSyncStore so that line 194 calls self.sync_store.update(broker, info)\n\nThat way container-sync specific names such as X-Container-Sync-To do not leak into this replicator module.","commit_id":"fa6a27b075651a6a731715e324121d36f761c083"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"377478c9669b0ab3185759dfb02a536e80425103","unresolved":false,"context_lines":[{"line_number":210,"context_line":"    def _update_sync_store(self, broker, info):"},{"line_number":211,"context_line":"        \"\"\""},{"line_number":212,"context_line":"        Udpates the sync db post replication"},{"line_number":213,"context_line":"        We do this here rather then in the server upon PUT/POST as in the"},{"line_number":214,"context_line":"        server we are not aware of the request vs. metadata timestamps"},{"line_number":215,"context_line":"        At any rate we need to do this update from one of the background"},{"line_number":216,"context_line":"        processes"}],"source_content_type":"text/x-python","patch_set":23,"id":"9a8ffd7b_89912d55","line":213,"updated":"2015-11-30 16:49:30.000000000","message":"nit s/then/than/","commit_id":"fa6a27b075651a6a731715e324121d36f761c083"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"377478c9669b0ab3185759dfb02a536e80425103","unresolved":false,"context_lines":[{"line_number":211,"context_line":"        \"\"\""},{"line_number":212,"context_line":"        Udpates the sync db post replication"},{"line_number":213,"context_line":"        We do this here rather then in the server upon PUT/POST as in the"},{"line_number":214,"context_line":"        server we are not aware of the request vs. metadata timestamps"},{"line_number":215,"context_line":"        At any rate we need to do this update from one of the background"},{"line_number":216,"context_line":"        processes"},{"line_number":217,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"9a8ffd7b_2751450f","line":214,"updated":"2015-11-30 16:49:30.000000000","message":"I didn\u0027t understand this comment about request vs metadata timestamps.\n\nDo you mean that this has to be done by a background process so that the sync-store is guaranteed to be eventually updated even if the original PUT/POST request to a replica may get lost? If so, could it not happen on both the original request and be checked by the replicator, which would make container-sync more responsive to the change in sync metadata (i.e. you would not need to wait for a replication run for container sync to pick up the change in sync metadata).","commit_id":"fa6a27b075651a6a731715e324121d36f761c083"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"377478c9669b0ab3185759dfb02a536e80425103","unresolved":false,"context_lines":[{"line_number":219,"context_line":"                       replicated\u0027"},{"line_number":220,"context_line":"        :param info: The info of the container"},{"line_number":221,"context_line":"        \"\"\""},{"line_number":222,"context_line":"        if \u0027X-Container-Sync-To\u0027 not in broker.metadata:"},{"line_number":223,"context_line":"            return"},{"line_number":224,"context_line":"        sync_to \u003d broker.metadata[\u0027X-Container-Sync-To\u0027][0]"},{"line_number":225,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"9a8ffd7b_cce5fedc","line":222,"updated":"2015-11-30 16:49:30.000000000","message":"The container will only be sync\u0027d if it has both sync-to and sync-key metadata, so this method should check for both before adding/removing to sync store. Otherwise you may still end up crawling unnecessary containers.","commit_id":"fa6a27b075651a6a731715e324121d36f761c083"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"87358319cbc5b280ea9034f3acc15966670ae78f","unresolved":false,"context_lines":[{"line_number":237,"context_line":"                broker.account \u003d\u003d MISPLACED_OBJECTS_ACCOUNT):"},{"line_number":238,"context_line":"            # this container shouldn\u0027t be here, make sure it\u0027s cleaned up"},{"line_number":239,"context_line":"            self.reconciler_cleanups[broker.container] \u003d broker"},{"line_number":240,"context_line":"            return"},{"line_number":241,"context_line":"        return super(ContainerReplicator, self).delete_db(broker)"},{"line_number":242,"context_line":""},{"line_number":243,"context_line":"    def replicate_reconcilers(self):"}],"source_content_type":"text/x-python","patch_set":23,"id":"9a8ffd7b_af785090","line":240,"updated":"2015-11-25 23:28:15.000000000","message":"I think we could preempt some of the broken link issues if the replicator removed the sync store entry after it removed the db here during rebalance cleanup.","commit_id":"fa6a27b075651a6a731715e324121d36f761c083"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"377478c9669b0ab3185759dfb02a536e80425103","unresolved":false,"context_lines":[{"line_number":237,"context_line":"                broker.account \u003d\u003d MISPLACED_OBJECTS_ACCOUNT):"},{"line_number":238,"context_line":"            # this container shouldn\u0027t be here, make sure it\u0027s cleaned up"},{"line_number":239,"context_line":"            self.reconciler_cleanups[broker.container] \u003d broker"},{"line_number":240,"context_line":"            return"},{"line_number":241,"context_line":"        return super(ContainerReplicator, self).delete_db(broker)"},{"line_number":242,"context_line":""},{"line_number":243,"context_line":"    def replicate_reconcilers(self):"}],"source_content_type":"text/x-python","patch_set":23,"id":"9a8ffd7b_8c129657","line":240,"in_reply_to":"9a8ffd7b_61c82f23","updated":"2015-11-30 16:49:30.000000000","message":"+1 but I think the link should be removed before the db so its gone before its target db. If the target db then fails to be deleted, the symlink will be re-created on next replicator pass.","commit_id":"fa6a27b075651a6a731715e324121d36f761c083"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"5f60c903f91eb02c518c902a86412cbfab62d4fe","unresolved":false,"context_lines":[{"line_number":237,"context_line":"                broker.account \u003d\u003d MISPLACED_OBJECTS_ACCOUNT):"},{"line_number":238,"context_line":"            # this container shouldn\u0027t be here, make sure it\u0027s cleaned up"},{"line_number":239,"context_line":"            self.reconciler_cleanups[broker.container] \u003d broker"},{"line_number":240,"context_line":"            return"},{"line_number":241,"context_line":"        return super(ContainerReplicator, self).delete_db(broker)"},{"line_number":242,"context_line":""},{"line_number":243,"context_line":"    def replicate_reconcilers(self):"}],"source_content_type":"text/x-python","patch_set":23,"id":"9a8ffd7b_61c82f23","line":240,"in_reply_to":"9a8ffd7b_af785090","updated":"2015-11-26 07:51:00.000000000","message":"Yep, I see. So you basically suggest to add here:\nself.sync_store.remove_synced_container(broker, False)\n\nI wonder whether having this here, we can get rid of the stuff added to container_sync and leave there the original error handling.","commit_id":"fa6a27b075651a6a731715e324121d36f761c083"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":252,"context_line":"    def run_once(self, *args, **kwargs):"},{"line_number":253,"context_line":"        self.reconciler_containers \u003d {}"},{"line_number":254,"context_line":"        self.reconciler_cleanups \u003d {}"},{"line_number":255,"context_line":"        self.sync_store \u003d ContainerSyncStore(self.root,"},{"line_number":256,"context_line":"                                             self.logger,"},{"line_number":257,"context_line":"                                             self.mount_check)"},{"line_number":258,"context_line":"        rv \u003d super(ContainerReplicator, self).run_once(*args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_8c1e556e","line":255,"updated":"2015-12-21 17:01:17.000000000","message":"could this be set up in an __init__ method ? I think that might simplify some of the unit test setup","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4b85b8ceb32be975e46a15dbe0325c352af16e1e","unresolved":false,"context_lines":[{"line_number":220,"context_line":"            return"},{"line_number":221,"context_line":"        try:"},{"line_number":222,"context_line":"            # DB is going to get deleted. Be preemptive about it"},{"line_number":223,"context_line":"            self.sync_store.remove_synced_container(broker)"},{"line_number":224,"context_line":"        except Exception:"},{"line_number":225,"context_line":"            self.logger.exception(\u0027Failed to remove sync_store entry %s\u0027 %"},{"line_number":226,"context_line":"                                  broker.db_file)"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_ebb07745","line":223,"updated":"2016-01-05 16:58:35.000000000","message":"ok. here we are pre-emptive, in server.py we are not. Because here we really are going to delete the db file.","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"1894a6f77a2cee4c903ef90cbea78e7e8b593469","unresolved":false,"context_lines":[{"line_number":220,"context_line":"            return"},{"line_number":221,"context_line":"        try:"},{"line_number":222,"context_line":"            # DB is going to get deleted. Be preemptive about it"},{"line_number":223,"context_line":"            self.sync_store.remove_synced_container(broker)"},{"line_number":224,"context_line":"        except Exception:"},{"line_number":225,"context_line":"            self.logger.exception(\u0027Failed to remove sync_store entry %s\u0027 %"},{"line_number":226,"context_line":"                                  broker.db_file)"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_9638cdef","line":223,"in_reply_to":"fa69d971_ebb07745","updated":"2016-01-05 21:23:33.000000000","message":"As far as I understand by preemptive you mean remove the symlink before removing the db. So far I meant remove the symlink by a background process (e.g. in replicator) rather than synchronously upon user request.","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"}],"swift/container/server.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7a10ba36645654a9bc283d36870616a28d09361d","unresolved":false,"context_lines":[{"line_number":283,"context_line":"            existed \u003d Timestamp(broker.get_info()[\u0027put_timestamp\u0027]) and \\"},{"line_number":284,"context_line":"                not broker.is_deleted()"},{"line_number":285,"context_line":"            broker.delete_db(req_timestamp.internal)"},{"line_number":286,"context_line":"            if not broker.is_deleted():"},{"line_number":287,"context_line":"                return HTTPConflict(request\u003dreq)"},{"line_number":288,"context_line":"            resp \u003d self.account_update(req, account, container, broker)"},{"line_number":289,"context_line":"            if resp:"}],"source_content_type":"text/x-python","patch_set":26,"id":"7a740942_359d190a","line":286,"updated":"2015-12-15 17:38:10.000000000","message":"is there any reason not to call _update_sync_store here? I think that the sync daemon does not sync once the container is marked as deleted, so no need for the symlink to exist.","commit_id":"b57a7a810d7fd635d7f2107d147ee0ab72e01216"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"399151b9656704a7e2ee6329e40bd896a2cc4b23","unresolved":false,"context_lines":[{"line_number":283,"context_line":"            existed \u003d Timestamp(broker.get_info()[\u0027put_timestamp\u0027]) and \\"},{"line_number":284,"context_line":"                not broker.is_deleted()"},{"line_number":285,"context_line":"            broker.delete_db(req_timestamp.internal)"},{"line_number":286,"context_line":"            if not broker.is_deleted():"},{"line_number":287,"context_line":"                return HTTPConflict(request\u003dreq)"},{"line_number":288,"context_line":"            resp \u003d self.account_update(req, account, container, broker)"},{"line_number":289,"context_line":"            if resp:"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a710552_bd274211","line":286,"in_reply_to":"7a740942_359d190a","updated":"2015-12-16 06:44:37.000000000","message":"Yep, I assume you meant to do so after line 287","commit_id":"b57a7a810d7fd635d7f2107d147ee0ab72e01216"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d3272fdaa3aa986b2e5ab3997a548cf06ecba54f","unresolved":false,"context_lines":[{"line_number":393,"context_line":"                        broker.metadata[\u0027X-Container-Sync-To\u0027][0]:"},{"line_number":394,"context_line":"                    broker.set_x_container_sync_points(-1, -1)"},{"line_number":395,"context_line":"            broker.update_metadata(metadata, validate_metadata\u003dTrue)"},{"line_number":396,"context_line":"            self._update_sync_store(broker, \u0027PUT\u0027)"},{"line_number":397,"context_line":"            resp \u003d self.account_update(req, account, container, broker)"},{"line_number":398,"context_line":"            if resp:"},{"line_number":399,"context_line":"                return resp"}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_c219c4d8","line":396,"updated":"2015-12-18 19:01:55.000000000","message":"small thing, but I think this line ^^ could be preceded by\n\n  if metadata:\n\ni.e. no change -\u003e do nothing\n\nBut the way it is does mean that should the sysnc stor eupdate fail on a PUT, it gets re-tried on the next one. It just struck me as different w.r.t. the POST method.\n\nIn fact i think all of lines 390-396 could be under that condition, similar to the POST method, but thats out of scope for this patch.","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"133e8a97d28c0e1b328e9f7ade6cfdeeae57ae8e","unresolved":false,"context_lines":[{"line_number":393,"context_line":"                        broker.metadata[\u0027X-Container-Sync-To\u0027][0]:"},{"line_number":394,"context_line":"                    broker.set_x_container_sync_points(-1, -1)"},{"line_number":395,"context_line":"            broker.update_metadata(metadata, validate_metadata\u003dTrue)"},{"line_number":396,"context_line":"            self._update_sync_store(broker, \u0027PUT\u0027)"},{"line_number":397,"context_line":"            resp \u003d self.account_update(req, account, container, broker)"},{"line_number":398,"context_line":"            if resp:"},{"line_number":399,"context_line":"                return resp"}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_4325d6be","line":396,"in_reply_to":"3a7e1126_c219c4d8","updated":"2015-12-19 00:03:21.000000000","message":"Done. I had to hold my self back hard to do the conservative choice and put only the call to _update_sync_store under the condition :-)","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4b85b8ceb32be975e46a15dbe0325c352af16e1e","unresolved":false,"context_lines":[{"line_number":287,"context_line":"            broker.delete_db(req_timestamp.internal)"},{"line_number":288,"context_line":"            if not broker.is_deleted():"},{"line_number":289,"context_line":"                return HTTPConflict(request\u003dreq)"},{"line_number":290,"context_line":"            self._update_sync_store(broker, \u0027DELETE\u0027)"},{"line_number":291,"context_line":"            resp \u003d self.account_update(req, account, container, broker)"},{"line_number":292,"context_line":"            if resp:"},{"line_number":293,"context_line":"                return resp"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_4bce63b5","line":290,"updated":"2016-01-05 16:58:35.000000000","message":"(comment only) in replicator we remove symlink before deleting db, here we do it after, which increases the possibility of a stale symlink. But the sync process does handle stale links, so I guess its ok.\n\nIn fact, it must be in this order here because broker.delete_db does not actually delete the db file (cf. replicator cas) and does not necessarily result in the container being deleted - it could have a later put timestamp.","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"1894a6f77a2cee4c903ef90cbea78e7e8b593469","unresolved":false,"context_lines":[{"line_number":287,"context_line":"            broker.delete_db(req_timestamp.internal)"},{"line_number":288,"context_line":"            if not broker.is_deleted():"},{"line_number":289,"context_line":"                return HTTPConflict(request\u003dreq)"},{"line_number":290,"context_line":"            self._update_sync_store(broker, \u0027DELETE\u0027)"},{"line_number":291,"context_line":"            resp \u003d self.account_update(req, account, container, broker)"},{"line_number":292,"context_line":"            if resp:"},{"line_number":293,"context_line":"                return resp"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_ac790084","line":290,"in_reply_to":"fa69d971_4bce63b5","updated":"2016-01-05 21:23:33.000000000","message":"Right.","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"}],"swift/container/sync.py":[{"author":{"_account_id":17156,"name":"Hamdi Roumani","email":"roumani@ca.ibm.com","username":"hroumani"},"change_message_id":"4c22e3e0e2a56883a4562458f97797551fad55d5","unresolved":false,"context_lines":[{"line_number":252,"context_line":"            begin \u003d time()"},{"line_number":253,"context_line":"            for path in self.sync_db.synced_containers():"},{"line_number":254,"context_line":"                self.container_sync(path)"},{"line_number":255,"context_line":"                if time() - self.reported \u003e\u003d 3600:  # once an hour"},{"line_number":256,"context_line":"                    self.report()"},{"line_number":257,"context_line":"            elapsed \u003d time() - begin"},{"line_number":258,"context_line":"            if elapsed \u003c self.interval:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_0902c709","line":255,"updated":"2015-07-26 15:38:49.000000000","message":"Should the time value (hour) be configurable through a conf setting ? (not directly related to your change).","commit_id":"d3882aa89aa2652cebc9bbf517de2d0a9e2344bb"},{"author":{"_account_id":17156,"name":"Hamdi Roumani","email":"roumani@ca.ibm.com","username":"hroumani"},"change_message_id":"4c22e3e0e2a56883a4562458f97797551fad55d5","unresolved":false,"context_lines":[{"line_number":389,"context_line":"            # In case the exception has to do with"},{"line_number":390,"context_line":"            # the db file not there, delete it from"},{"line_number":391,"context_line":"            # the sync_db."},{"line_number":392,"context_line":"            if not os.path.exists(path):"},{"line_number":393,"context_line":"                self.sync_db.remove_synced_container(broker)"},{"line_number":394,"context_line":"            self.container_failures +\u003d 1"},{"line_number":395,"context_line":"            self.logger.increment(\u0027failures\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_293463dc","line":392,"updated":"2015-07-26 15:38:49.000000000","message":"For clarity I was going to suggest explicitly catching IOError but given you\u0027ll probably still want to check if said path exists there\u0027s probably no point. Also it seems its not easy to determine the exception type (or you could add another except clause before this generic one, but then would need to duplicate the code after).","commit_id":"d3882aa89aa2652cebc9bbf517de2d0a9e2344bb"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a20455f7416ea5176823547eff8b278eeaa5e1d1","unresolved":false,"context_lines":[{"line_number":390,"context_line":"            # the db file not there, delete it from"},{"line_number":391,"context_line":"            # the sync_store."},{"line_number":392,"context_line":"            if not os.path.exists(path):"},{"line_number":393,"context_line":"                self.sync_store.remove_synced_container(broker)"},{"line_number":394,"context_line":"            self.container_failures +\u003d 1"},{"line_number":395,"context_line":"            self.logger.increment(\u0027failures\u0027)"},{"line_number":396,"context_line":"            self.logger.exception(_(\u0027ERROR Syncing %s\u0027),"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_f4834301","line":393,"updated":"2015-08-18 19:15:04.000000000","message":"add a specific handler for OSError please, check for ENOENT or whatever the error code is when the link is invalid.  Consider this traceback after a rebalance:\n\n    container-6021: ERROR Syncing /srv/node2/sdb2/.sync_containers/42/b7c/0ab88e8f240309b63e3711357bbc5b7c/0ab88e8f240309b63e3711357bbc5b7c.db: \n    Traceback (most recent call last):\n      File \"/vagrant/swift/swift/container/sync.py\", line 308, in container_sync\n        info \u003d broker.get_info()\n      File \"/vagrant/swift/swift/container/backend.py\", line 399, in get_info\n        with self.get() as conn:\n      File \"/usr/lib/python2.7/contextlib.py\", line 17, in __enter__\n        return self.gen.next()\n      File \"/vagrant/swift/swift/common/db.py\", line 364, in get\n        raise DatabaseConnectionError(self.db_file, \"DB doesn\u0027t exist\")\n    DatabaseConnectionError: DB connection error (/srv/node2/sdb2/.sync_containers/42/b7c/0ab88e8f240309b63e3711357bbc5b7c/0ab88e8f240309b63e3711357bbc5b7c.db, 0):\n    DB doesn\u0027t exist\n\nI think path here might be the link?  path.exists is actually just verifying the link is there?","commit_id":"aefd20882642662b4f7deeb19ddfa9617cc8c2ff"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"3adb96385ea732fefdca2b1763dd49d19885f8f2","unresolved":false,"context_lines":[{"line_number":389,"context_line":"            # In case the exception has to do with"},{"line_number":390,"context_line":"            # the db file not there, delete it from"},{"line_number":391,"context_line":"            # the sync_store."},{"line_number":392,"context_line":"            if not os.path.exists(path):"},{"line_number":393,"context_line":"                try:"},{"line_number":394,"context_line":"                    self.sync_store.remove_synced_container(broker)"},{"line_number":395,"context_line":"                except OSError:"}],"source_content_type":"text/x-python","patch_set":11,"id":"da20952f_c9ade6eb","line":392,"updated":"2015-09-08 19:00:17.000000000","message":"is path here the path to symlink or the path to the database?\n\nIf the path does not exist should be be incrementing \"failures\" or is this just a normal symlinks out of sync situation?\n\nI still think it\u0027s somehow awkward to catch an generic exception, and then try to narrow it down when python has native support for multiple except blocks (waterfall clauses) with the final \"except Exception\" typically reserved for the Pokemon/default exception case.\n\nOSError can be a little trixy, because you may only want to handle the specific errno, but I still prefer it:\n\n    try:\n        try:\n            do_something()\n        excpet IOError as e:\n            if e.errno !\u003d errno.ENOENT:\n                raise\n            eafp_cleanup()\n    except Exception as e:\n        logging.exception(\u0027something bad trying to do_something\u0027)","commit_id":"fcd483ad6bf1ab70d2e2ef26ad4b363863f73837"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ce02d6f0770307fd88355bfbb4314bfce8b27fbb","unresolved":false,"context_lines":[{"line_number":310,"context_line":"                info \u003d broker.get_info()"},{"line_number":311,"context_line":"            except DatabaseConnectionError:"},{"line_number":312,"context_line":"                if os.path.exists(path) is True:"},{"line_number":313,"context_line":"                    self.sync_store.remove_synced_container(broker)"},{"line_number":314,"context_line":"                raise"},{"line_number":315,"context_line":""},{"line_number":316,"context_line":"            x, nodes \u003d self.container_ring.get_nodes(info[\u0027account\u0027],"}],"source_content_type":"text/x-python","patch_set":22,"id":"ba8a016a_814be186","line":313,"updated":"2015-11-17 22:54:27.000000000","message":"I think the idea here is to clean up the symlink\n\n... except I\u0027m pretty sure path here is the path to the symlink, not the db itself.  I\u0027m would think the idea was to make the \"remove_synced_container\" conditional on the database itself not existing - but instead it seems to check if the symlink exists before removing it (which is still racy)","commit_id":"d955f462a4fdda2223281a472d2bfd100d24081e"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"9db18081c138967dc6b0c8cd2ed483c658a8a510","unresolved":false,"context_lines":[{"line_number":310,"context_line":"                info \u003d broker.get_info()"},{"line_number":311,"context_line":"            except DatabaseConnectionError:"},{"line_number":312,"context_line":"                if os.path.exists(path) is True:"},{"line_number":313,"context_line":"                    self.sync_store.remove_synced_container(broker)"},{"line_number":314,"context_line":"                raise"},{"line_number":315,"context_line":""},{"line_number":316,"context_line":"            x, nodes \u003d self.container_ring.get_nodes(info[\u0027account\u0027],"}],"source_content_type":"text/x-python","patch_set":22,"id":"ba8a016a_f3fe3137","line":313,"in_reply_to":"ba8a016a_814be186","updated":"2015-11-18 07:57:21.000000000","message":"Indeed the path here is the symlink path. Note, however, that we do this only when getting DatabaseConnectionError which is pretty much an indication that the .db is not there. Having said that this is an opportunistic cleanup that is in a sense redundant as the replicator would clean all symlinks of deleted dbs. See https://review.openstack.org/#/c/205803/22/swift/container/replicator.py line 210. Am I missing something here?","commit_id":"d955f462a4fdda2223281a472d2bfd100d24081e"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"bc000330a9c91ed211cb9a6639c3a7f13457793d","unresolved":false,"context_lines":[{"line_number":310,"context_line":"                info \u003d broker.get_info()"},{"line_number":311,"context_line":"            except DatabaseConnectionError:"},{"line_number":312,"context_line":"                if os.path.exists(path) is True:"},{"line_number":313,"context_line":"                    self.sync_store.remove_synced_container(broker)"},{"line_number":314,"context_line":"                raise"},{"line_number":315,"context_line":""},{"line_number":316,"context_line":"            x, nodes \u003d self.container_ring.get_nodes(info[\u0027account\u0027],"}],"source_content_type":"text/x-python","patch_set":22,"id":"ba8a016a_637b168d","line":313,"in_reply_to":"ba8a016a_a7d5aa1c","updated":"2015-11-19 13:38:35.000000000","message":"Absolutly!","commit_id":"d955f462a4fdda2223281a472d2bfd100d24081e"},{"author":{"_account_id":995,"name":"David Goetz","email":"david.goetz@rackspace.com","username":"david-goetz"},"change_message_id":"7e18713d1db533fe9ab8f2799734055600e245c9","unresolved":false,"context_lines":[{"line_number":310,"context_line":"                info \u003d broker.get_info()"},{"line_number":311,"context_line":"            except DatabaseConnectionError:"},{"line_number":312,"context_line":"                if os.path.exists(path) is True:"},{"line_number":313,"context_line":"                    self.sync_store.remove_synced_container(broker)"},{"line_number":314,"context_line":"                raise"},{"line_number":315,"context_line":""},{"line_number":316,"context_line":"            x, nodes \u003d self.container_ring.get_nodes(info[\u0027account\u0027],"}],"source_content_type":"text/x-python","patch_set":22,"id":"ba8a016a_a7d5aa1c","line":313,"in_reply_to":"ba8a016a_f3fe3137","updated":"2015-11-18 17:27:40.000000000","message":"should this be:\n\nif os.path.lexists(path):\n\n?","commit_id":"d955f462a4fdda2223281a472d2bfd100d24081e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"377478c9669b0ab3185759dfb02a536e80425103","unresolved":false,"context_lines":[{"line_number":189,"context_line":"            a.strip()"},{"line_number":190,"context_line":"            for a in conf.get(\u0027sync_proxy\u0027, \u0027\u0027).split(\u0027,\u0027)"},{"line_number":191,"context_line":"            if a.strip()]"},{"line_number":192,"context_line":"        #: ContainerSyncDB instance for iterating over synced containers"},{"line_number":193,"context_line":"        self.sync_store \u003d ContainerSyncStore(self.devices, self.logger)"},{"line_number":194,"context_line":"        #: Number of containers with sync turned on that were successfully"},{"line_number":195,"context_line":"        #: synced."}],"source_content_type":"text/x-python","patch_set":23,"id":"9a8ffd7b_bb80f2f8","line":192,"updated":"2015-11-30 16:49:30.000000000","message":"nit comment should refer to ContainerSyncStore?","commit_id":"fa6a27b075651a6a731715e324121d36f761c083"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"377478c9669b0ab3185759dfb02a536e80425103","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        self.container_deletes \u003d 0"},{"line_number":199,"context_line":"        #: Number of successful PUTs triggered."},{"line_number":200,"context_line":"        self.container_puts \u003d 0"},{"line_number":201,"context_line":"        #: Number of containers that didn\u0027t have sync turned on."},{"line_number":202,"context_line":"        self.container_skips \u003d 0"},{"line_number":203,"context_line":"        #: Number of containers that had a failure of some type."},{"line_number":204,"context_line":"        self.container_failures \u003d 0"}],"source_content_type":"text/x-python","patch_set":23,"id":"9a8ffd7b_31e423ab","line":201,"updated":"2015-11-30 16:49:30.000000000","message":"the meaning of the logged value container_skips has changed, so this comment should be updated. It\u0027s now (slightly strange) the number of containers that have sync-to but not sync-key setup (you can guess that i discovered this while trying this patch and forgetting to set my sync-keys :)\n\nIn fact, it may be that \"skipped\" is now redundant.","commit_id":"fa6a27b075651a6a731715e324121d36f761c083"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"87358319cbc5b280ea9034f3acc15966670ae78f","unresolved":false,"context_lines":[{"line_number":309,"context_line":"            try:"},{"line_number":310,"context_line":"                info \u003d broker.get_info()"},{"line_number":311,"context_line":"            except DatabaseConnectionError:"},{"line_number":312,"context_line":"                if os.path.lexists(path) is True:"},{"line_number":313,"context_line":"                    self.sync_store.remove_synced_container(broker, True)"},{"line_number":314,"context_line":"                raise"},{"line_number":315,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"9a8ffd7b_8cd1da59","line":312,"updated":"2015-11-25 23:28:15.000000000","message":"I think this chart demonstrates there\u0027s some information gap with this condition:\n\nhttps://gist.github.com/clayg/e188ed0063e6d5d4937a\n\nIn fact I\u0027m not sure any single call to exists or lexists can be sufficient so say we understand the state?\n\n`if lexists and not exists` on the link path should do the trick, but then again I think it could also just be a permissions thing?  OTOH if we can assume the DatabaseConnectionError was due to some sort of file not exists problem - then maybe the lexists check is sufficient?\n\nI\u0027m waffling.","commit_id":"fa6a27b075651a6a731715e324121d36f761c083"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"5f60c903f91eb02c518c902a86412cbfab62d4fe","unresolved":false,"context_lines":[{"line_number":309,"context_line":"            try:"},{"line_number":310,"context_line":"                info \u003d broker.get_info()"},{"line_number":311,"context_line":"            except DatabaseConnectionError:"},{"line_number":312,"context_line":"                if os.path.lexists(path) is True:"},{"line_number":313,"context_line":"                    self.sync_store.remove_synced_container(broker, True)"},{"line_number":314,"context_line":"                raise"},{"line_number":315,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"9a8ffd7b_d43bd7cf","line":312,"in_reply_to":"9a8ffd7b_8cd1da59","updated":"2015-11-26 07:51:00.000000000","message":"Nice chart! Yeah, so it proves the point that if we cannot trust the DatabaseConnectionError to truley reflect the db file is not there we need more then lexist.","commit_id":"fa6a27b075651a6a731715e324121d36f761c083"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"87358319cbc5b280ea9034f3acc15966670ae78f","unresolved":false,"context_lines":[{"line_number":311,"context_line":"            except DatabaseConnectionError:"},{"line_number":312,"context_line":"                if os.path.lexists(path) is True:"},{"line_number":313,"context_line":"                    self.sync_store.remove_synced_container(broker, True)"},{"line_number":314,"context_line":"                raise"},{"line_number":315,"context_line":""},{"line_number":316,"context_line":"            x, nodes \u003d self.container_ring.get_nodes(info[\u0027account\u0027],"},{"line_number":317,"context_line":"                                                     info[\u0027container\u0027])"}],"source_content_type":"text/x-python","patch_set":23,"id":"9a8ffd7b_cc5602d2","line":314,"updated":"2015-11-25 23:28:15.000000000","message":"If we\u0027ve assumed the reason we got the DataBaseConnectionError was because the database was rebalanced/replicated off this node and the symlink should now be cleaned up - then this \"error\" is *not* an exceptional case and we should *not* log a an un-handled exception traceback.\n\nI\u0027m pretty sure the re-raise should be in an else block and the happy path clean up should just return with no fuss or noise - maybe a debug message.","commit_id":"fa6a27b075651a6a731715e324121d36f761c083"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"5f60c903f91eb02c518c902a86412cbfab62d4fe","unresolved":false,"context_lines":[{"line_number":311,"context_line":"            except DatabaseConnectionError:"},{"line_number":312,"context_line":"                if os.path.lexists(path) is True:"},{"line_number":313,"context_line":"                    self.sync_store.remove_synced_container(broker, True)"},{"line_number":314,"context_line":"                raise"},{"line_number":315,"context_line":""},{"line_number":316,"context_line":"            x, nodes \u003d self.container_ring.get_nodes(info[\u0027account\u0027],"},{"line_number":317,"context_line":"                                                     info[\u0027container\u0027])"}],"source_content_type":"text/x-python","patch_set":23,"id":"9a8ffd7b_34802346","line":314,"in_reply_to":"9a8ffd7b_cc5602d2","updated":"2015-11-26 07:51:00.000000000","message":"Good point! I absolutely buy https://gist.github.com/clayg/5f6784e06c224413743a\nThe remaining question is whether we want to disregard this error here altogether, if we get more \"preemptive\" in the replicator as you suggest","commit_id":"fa6a27b075651a6a731715e324121d36f761c083"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7a10ba36645654a9bc283d36870616a28d09361d","unresolved":false,"context_lines":[{"line_number":199,"context_line":"        #: Number of successful PUTs triggered."},{"line_number":200,"context_line":"        self.container_puts \u003d 0"},{"line_number":201,"context_line":"        #: Number of containers whose sync has been turned off, but"},{"line_number":202,"context_line":"        #: are not yet cleared from the sync store."},{"line_number":203,"context_line":"        self.container_skips \u003d 0"},{"line_number":204,"context_line":"        #: Number of containers that had a failure of some type."},{"line_number":205,"context_line":"        self.container_failures \u003d 0"}],"source_content_type":"text/x-python","patch_set":26,"id":"7a740942_7465dc42","line":202,"updated":"2015-12-15 17:38:10.000000000","message":"see comment in server.py - why would these not have been cleared from sync store?\n\n(actually, answering my own question, its possible that even if the server attempted to remove the symlinks during a DELETE, that operation could fail, leaving a symlink to a db with deleted\u003dtrue.)","commit_id":"b57a7a810d7fd635d7f2107d147ee0ab72e01216"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"399151b9656704a7e2ee6329e40bd896a2cc4b23","unresolved":false,"context_lines":[{"line_number":199,"context_line":"        #: Number of successful PUTs triggered."},{"line_number":200,"context_line":"        self.container_puts \u003d 0"},{"line_number":201,"context_line":"        #: Number of containers whose sync has been turned off, but"},{"line_number":202,"context_line":"        #: are not yet cleared from the sync store."},{"line_number":203,"context_line":"        self.container_skips \u003d 0"},{"line_number":204,"context_line":"        #: Number of containers that had a failure of some type."},{"line_number":205,"context_line":"        self.container_failures \u003d 0"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a710552_382da046","line":202,"in_reply_to":"7a740942_7465dc42","updated":"2015-12-16 06:44:37.000000000","message":"I think there is also a small chance of a race, where the DELETE arrives right after the iterator hands the pah to the sync daemon","commit_id":"b57a7a810d7fd635d7f2107d147ee0ab72e01216"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":315,"context_line":"                if str(db_err).endswith(\"DB doesn\u0027t exist\"):"},{"line_number":316,"context_line":"                    try:"},{"line_number":317,"context_line":"                        self.sync_store.remove_synced_container(broker)"},{"line_number":318,"context_line":"                    except OSError as e:"},{"line_number":319,"context_line":"                        if e.errno !\u003d errno.ENOENT:"},{"line_number":320,"context_line":"                            raise"},{"line_number":321,"context_line":"                    self.logger.debug(\u0027Removed stale sync reference %s\u0027 % path)"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_1eb4d232","line":318,"updated":"2015-12-21 17:01:17.000000000","message":"this duplicates the check already made in remove_synced_container(), so i think the try/except is not needed here.","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":315,"context_line":"                if str(db_err).endswith(\"DB doesn\u0027t exist\"):"},{"line_number":316,"context_line":"                    try:"},{"line_number":317,"context_line":"                        self.sync_store.remove_synced_container(broker)"},{"line_number":318,"context_line":"                    except OSError as e:"},{"line_number":319,"context_line":"                        if e.errno !\u003d errno.ENOENT:"},{"line_number":320,"context_line":"                            raise"},{"line_number":321,"context_line":"                    self.logger.debug(\u0027Removed stale sync reference %s\u0027 % path)"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_1a18fb1c","line":318,"in_reply_to":"3a7e1126_1eb4d232","updated":"2016-01-01 21:44:24.000000000","message":"yep. Removed.","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":317,"context_line":"                        self.sync_store.remove_synced_container(broker)"},{"line_number":318,"context_line":"                    except OSError as e:"},{"line_number":319,"context_line":"                        if e.errno !\u003d errno.ENOENT:"},{"line_number":320,"context_line":"                            raise"},{"line_number":321,"context_line":"                    self.logger.debug(\u0027Removed stale sync reference %s\u0027 % path)"},{"line_number":322,"context_line":"                    return"},{"line_number":323,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_31e1bf2d","line":320,"updated":"2015-12-21 17:01:17.000000000","message":"Having a raise here loses causes the error to be logged and increment container_failures - do we want that to happen when the error is in cleaning up a stale reference? Or should it be logged *here* and then we return, so bypassing the container_failures counting and logging at line 404?","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":317,"context_line":"                        self.sync_store.remove_synced_container(broker)"},{"line_number":318,"context_line":"                    except OSError as e:"},{"line_number":319,"context_line":"                        if e.errno !\u003d errno.ENOENT:"},{"line_number":320,"context_line":"                            raise"},{"line_number":321,"context_line":"                    self.logger.debug(\u0027Removed stale sync reference %s\u0027 % path)"},{"line_number":322,"context_line":"                    return"},{"line_number":323,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_5a00b334","line":320,"in_reply_to":"3a7e1126_31e1bf2d","updated":"2016-01-01 21:44:24.000000000","message":"right. Well, it is removed now...","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":319,"context_line":"                        if e.errno !\u003d errno.ENOENT:"},{"line_number":320,"context_line":"                            raise"},{"line_number":321,"context_line":"                    self.logger.debug(\u0027Removed stale sync reference %s\u0027 % path)"},{"line_number":322,"context_line":"                    return"},{"line_number":323,"context_line":"                raise"},{"line_number":324,"context_line":""},{"line_number":325,"context_line":"            x, nodes \u003d self.container_ring.get_nodes(info[\u0027account\u0027],"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_31fc5f6f","line":322,"updated":"2015-12-21 17:01:17.000000000","message":"Hmmm, so we return here rather than allow the \noriginal DatabaseConnectionError to be handled at line 404? Is that because we only expect this \"DB doesn\u0027t exist\" error to occur as a result of the symlinks getting stale w.r.t the db? So we don\u0027t consider this to be an error in sync\u0027ing the db, so not logged.\n\nMaybe add a small comment before the return e.g. \"Don\u0027t log stale reference as a sync error, just return\"","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":319,"context_line":"                        if e.errno !\u003d errno.ENOENT:"},{"line_number":320,"context_line":"                            raise"},{"line_number":321,"context_line":"                    self.logger.debug(\u0027Removed stale sync reference %s\u0027 % path)"},{"line_number":322,"context_line":"                    return"},{"line_number":323,"context_line":"                raise"},{"line_number":324,"context_line":""},{"line_number":325,"context_line":"            x, nodes \u003d self.container_ring.get_nodes(info[\u0027account\u0027],"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_fa4e47c0","line":322,"in_reply_to":"3a7e1126_31fc5f6f","updated":"2016-01-01 21:44:24.000000000","message":"I think you have got a good point here. Before we were preemptive with updating the links (on delete/put/post as your review suggested) the cases were we could have a stale link were much more common. Now, these cases are rare and so I gave up the return so as to have the handling done in 404.","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4b85b8ceb32be975e46a15dbe0325c352af16e1e","unresolved":false,"context_lines":[{"line_number":318,"context_line":"            # since in most cases the db should be there."},{"line_number":319,"context_line":"            try:"},{"line_number":320,"context_line":"                info \u003d broker.get_info()"},{"line_number":321,"context_line":"            except DatabaseConnectionError as db_err:"},{"line_number":322,"context_line":"                if str(db_err).endswith(\"DB doesn\u0027t exist\"):"},{"line_number":323,"context_line":"                    self.sync_store.remove_synced_container(broker)"},{"line_number":324,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_4b166356","line":321,"updated":"2016-01-05 16:58:35.000000000","message":"do we have a test for stale symlink being handled here? Yes we do :)\n\nNote that the broker is initialised outside of this try/except. Currently the init method does not attempt to access the db so the scope of the try/except is appropriate, but a test should verify that to prevent a regression. i.e. test that a stale symlink gets removed when the sync process runs.","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"1894a6f77a2cee4c903ef90cbea78e7e8b593469","unresolved":false,"context_lines":[{"line_number":318,"context_line":"            # since in most cases the db should be there."},{"line_number":319,"context_line":"            try:"},{"line_number":320,"context_line":"                info \u003d broker.get_info()"},{"line_number":321,"context_line":"            except DatabaseConnectionError as db_err:"},{"line_number":322,"context_line":"                if str(db_err).endswith(\"DB doesn\u0027t exist\"):"},{"line_number":323,"context_line":"                    self.sync_store.remove_synced_container(broker)"},{"line_number":324,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_2cef50ea","line":321,"in_reply_to":"fa69d971_4b166356","updated":"2016-01-05 21:23:33.000000000","message":"yep. good point.","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"}],"swift/container/sync_db.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1a4b3c91795b444c3534c75778803ffe03bee8c7","unresolved":false,"context_lines":[{"line_number":23,"context_line":"SYNC_DATADIR \u003d \u0027.sync_containers\u0027"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"class ContainerSyncDb(object):"},{"line_number":27,"context_line":"    \"\"\""},{"line_number":28,"context_line":"    Database for local containers that needs to be synced."},{"line_number":29,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_4c38afa0","line":26,"updated":"2015-07-27 18:38:02.000000000","message":"I don\u0027t think this is a great abstract name for the concept - especially given that this implementation uses a filesystem directory - which I wouldn\u0027t normally call a database - here the interface would work well with any k/v like store.\n\nI think a \"database\" is a specific case of the more general \"data store\".\n\nContainerSyncStore - ContainerSyncContainer - ContainerSyncSet - ContainerSyncTracker - ContainerSyncDBTracker - SyncDBTracker - Recorder, Watcher, Keeper - SyncDataStore?\n\nmeh. Just not Db.  We have to many things named Db that *are* databases to have a thing named Db that is not a database.","commit_id":"d3882aa89aa2652cebc9bbf517de2d0a9e2344bb"},{"author":{"_account_id":17156,"name":"Hamdi Roumani","email":"roumani@ca.ibm.com","username":"hroumani"},"change_message_id":"4c22e3e0e2a56883a4562458f97797551fad55d5","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        suffix \u003d path[path.find(DATADIR) + len(DATADIR) + 1:]"},{"line_number":58,"context_line":"        return (devices, device, suffix)"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    def _container_to_synced_container_path(self, path):"},{"line_number":61,"context_line":"        spath \u003d self._split_container_dir_path(path)"},{"line_number":62,"context_line":"        return os.path.join(spath[0], spath[1], SYNC_DATADIR, spath[2])"},{"line_number":63,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_06f1546a","line":60,"updated":"2015-07-26 15:38:49.000000000","message":"I find the function name slightly confusing, though struggling to come up with something better;  I wonder if _create_synced_container_path is a clearer at all ?\nAlso you use convention path \u003d\u003d dir and file \u003d\u003d file in a few places so maybe this is better as:  _create_synced_container_file","commit_id":"d3882aa89aa2652cebc9bbf517de2d0a9e2344bb"},{"author":{"_account_id":17156,"name":"Hamdi Roumani","email":"roumani@ca.ibm.com","username":"hroumani"},"change_message_id":"4c22e3e0e2a56883a4562458f97797551fad55d5","unresolved":false,"context_lines":[{"line_number":108,"context_line":""},{"line_number":109,"context_line":"        return True"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    def synced_containers(self):"},{"line_number":112,"context_line":"        \"\"\""},{"line_number":113,"context_line":"        Iterates over the list of synced containers"},{"line_number":114,"context_line":"        yielding the path of the container db"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_a63ae0e5","line":111,"updated":"2015-07-26 15:38:49.000000000","message":"Minor but for iterators is it clearer to use something like synced_container_iter / _generator ?","commit_id":"d3882aa89aa2652cebc9bbf517de2d0a9e2344bb"}],"swift/container/sync_store.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a20455f7416ea5176823547eff8b278eeaa5e1d1","unresolved":false,"context_lines":[{"line_number":20,"context_line":"from swift.common.utils import audit_location_generator"},{"line_number":21,"context_line":"from swift.container.backend import DATADIR"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"SYNC_DATADIR \u003d \u0027.sync_containers\u0027"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"class ContainerSyncStore(object):"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_343d2b8f","line":23,"updated":"2015-08-18 19:15:04.000000000","message":"i think I don\u0027t want hidden directory.  Do you have any reason you particularly keen on it?","commit_id":"aefd20882642662b4f7deeb19ddfa9617cc8c2ff"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a20455f7416ea5176823547eff8b278eeaa5e1d1","unresolved":false,"context_lines":[{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    def _container_to_synced_container_path(self, path):"},{"line_number":61,"context_line":"        spath \u003d self._split_container_dir_path(path)"},{"line_number":62,"context_line":"        return os.path.join(spath[0], spath[1], SYNC_DATADIR, spath[2])"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    def add_synced_container(self, broker):"},{"line_number":65,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"fa1b9901_d3e000fd","line":62,"updated":"2015-08-18 19:15:04.000000000","message":"I don\u0027t really think the partition dir is that helpful here?\n\n    /srv/node2/sdb2/.sync_containers/42/b7c/0ab88e8f240309b63e3711357bbc5b7c/0ab88e8f240309b63e3711357bbc5b7c.db\n\n... might be better just:\n\n    /srv/node2/sdb2/sync_containers/b7c/0ab88e8f240309b63e3711357bbc5b7c/0ab88e8f240309b63e3711357bbc5b7c.db\n\n... seems like it could save a some inodes - maybe it\u0027s trivial tho...","commit_id":"aefd20882642662b4f7deeb19ddfa9617cc8c2ff"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a20455f7416ea5176823547eff8b278eeaa5e1d1","unresolved":false,"context_lines":[{"line_number":68,"context_line":""},{"line_number":69,"context_line":"        :param broker: An instance of ContainerBroker representing the"},{"line_number":70,"context_line":"                       container to add."},{"line_number":71,"context_line":"        \"\"\""},{"line_number":72,"context_line":"        return self._add_synced_container(broker.db_file, broker.db_dir)"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    def _add_synced_container(self, container_file, container_path):"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_af11e653","line":71,"updated":"2015-08-18 19:15:04.000000000","message":"so this method gets called for every container every replication pass.  I wonder if this is case where LBYL with an os.path.exists is cheaper than the EAPF mkdirs \u0026 symlink calls?\n\n... in fact the life-cycle on this class seems to be rather long?  Do we really need to check that the symlink still exists for every pass of the replicator that encounters a container with x-container-sync-to?","commit_id":"aefd20882642662b4f7deeb19ddfa9617cc8c2ff"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a20455f7416ea5176823547eff8b278eeaa5e1d1","unresolved":false,"context_lines":[{"line_number":104,"context_line":"            os.unlink(sync_file)"},{"line_number":105,"context_line":"        except OSError as oserr:"},{"line_number":106,"context_line":"            if oserr.errno !\u003d errno.ENOENT:"},{"line_number":107,"context_line":"                return False"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"        return True"},{"line_number":110,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"fa1b9901_3335bcd9","line":107,"updated":"2015-08-18 19:15:04.000000000","message":"I don\u0027t like loosing the traceback here, I think ignoring ENOENT is find, but other wise it should raise","commit_id":"aefd20882642662b4f7deeb19ddfa9617cc8c2ff"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"3adb96385ea732fefdca2b1763dd49d19885f8f2","unresolved":false,"context_lines":[{"line_number":124,"context_line":"                                            mount_check\u003dFalse,"},{"line_number":125,"context_line":"                                            logger\u003dself.logger)"},{"line_number":126,"context_line":"        for path, device, partition in all_locs:"},{"line_number":127,"context_line":"            yield path"}],"source_content_type":"text/x-python","patch_set":11,"id":"da20952f_2944a219","line":127,"updated":"2015-09-08 19:00:17.000000000","message":"I almost wonder if it wouldn\u0027t be so bad to just stat the file here?  I mean, we know we\u0027re going to open it, and that will end up having to do some kind of read anyway to find the real path.  When the next system call goes to open it it\u0027ll still be another system call - but at that point it\u0027ll be cached filesystem metadata...\n\nI suppose the code we yield too will still need to be prepared to handle the race, so LBYL probably won\u0027t actually make it any simpler.\n\nNevermind!  Good work!","commit_id":"fcd483ad6bf1ab70d2e2ef26ad4b363863f73837"},{"author":{"_account_id":995,"name":"David Goetz","email":"david.goetz@rackspace.com","username":"david-goetz"},"change_message_id":"7e18713d1db533fe9ab8f2799734055600e245c9","unresolved":false,"context_lines":[{"line_number":51,"context_line":"        (\u0027/srv/node/\u0027, \u0027sdb/\u0027, part/.../*.db)"},{"line_number":52,"context_line":"        or more generally to:"},{"line_number":53,"context_line":"        (devices, device, part/.../*.db)"},{"line_number":54,"context_line":"        \"\"\""},{"line_number":55,"context_line":"        devices \u003d self.devices"},{"line_number":56,"context_line":"        device \u003d path[len(devices):path.find(DATADIR)]"},{"line_number":57,"context_line":"        suffix \u003d path[path.find(DATADIR) + len(DATADIR) + 1:]"}],"source_content_type":"text/x-python","patch_set":22,"id":"ba8a016a_e7d97223","line":54,"updated":"2015-11-18 17:27:40.000000000","message":"there isn\u0027t some library function somewhere that does this better?","commit_id":"d955f462a4fdda2223281a472d2bfd100d24081e"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"bc000330a9c91ed211cb9a6639c3a7f13457793d","unresolved":false,"context_lines":[{"line_number":51,"context_line":"        (\u0027/srv/node/\u0027, \u0027sdb/\u0027, part/.../*.db)"},{"line_number":52,"context_line":"        or more generally to:"},{"line_number":53,"context_line":"        (devices, device, part/.../*.db)"},{"line_number":54,"context_line":"        \"\"\""},{"line_number":55,"context_line":"        devices \u003d self.devices"},{"line_number":56,"context_line":"        device \u003d path[len(devices):path.find(DATADIR)]"},{"line_number":57,"context_line":"        suffix \u003d path[path.find(DATADIR) + len(DATADIR) + 1:]"}],"source_content_type":"text/x-python","patch_set":22,"id":"ba8a016a_3576a1a4","line":54,"in_reply_to":"ba8a016a_e7d97223","updated":"2015-11-19 13:38:35.000000000","message":"I searched common/utils.py and did not find one. Anywhere else I should look? \nDo you think a better approach would be to split the path using \u0027/\u0027 and work with that?","commit_id":"d955f462a4fdda2223281a472d2bfd100d24081e"},{"author":{"_account_id":995,"name":"David Goetz","email":"david.goetz@rackspace.com","username":"david-goetz"},"change_message_id":"7e18713d1db533fe9ab8f2799734055600e245c9","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        :param broker: An instance of ContainerBroker representing the"},{"line_number":70,"context_line":"                       container to add."},{"line_number":71,"context_line":"        \"\"\""},{"line_number":72,"context_line":"        return self._add_synced_container(broker.db_file, broker.db_dir)"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    def _add_synced_container(self, container_file, container_path):"},{"line_number":75,"context_line":"        sync_file \u003d self._container_to_synced_container_path(container_file)"}],"source_content_type":"text/x-python","patch_set":22,"id":"ba8a016a_07eef6b3","line":72,"updated":"2015-11-18 17:27:40.000000000","message":"another 1 line function - see below comment","commit_id":"d955f462a4fdda2223281a472d2bfd100d24081e"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"bc000330a9c91ed211cb9a6639c3a7f13457793d","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        :param broker: An instance of ContainerBroker representing the"},{"line_number":70,"context_line":"                       container to add."},{"line_number":71,"context_line":"        \"\"\""},{"line_number":72,"context_line":"        return self._add_synced_container(broker.db_file, broker.db_dir)"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    def _add_synced_container(self, container_file, container_path):"},{"line_number":75,"context_line":"        sync_file \u003d self._container_to_synced_container_path(container_file)"}],"source_content_type":"text/x-python","patch_set":22,"id":"ba8a016a_23298e37","line":72,"in_reply_to":"ba8a016a_07eef6b3","updated":"2015-11-19 13:38:35.000000000","message":"yep. fixed.","commit_id":"d955f462a4fdda2223281a472d2bfd100d24081e"},{"author":{"_account_id":995,"name":"David Goetz","email":"david.goetz@rackspace.com","username":"david-goetz"},"change_message_id":"7e18713d1db533fe9ab8f2799734055600e245c9","unresolved":false,"context_lines":[{"line_number":107,"context_line":"        \"\"\""},{"line_number":108,"context_line":"        return self._remove_synced_container(broker.db_file, broker.db_dir)"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    def _remove_synced_container(self, container_file, container_path):"},{"line_number":111,"context_line":"        sync_file \u003d self._container_to_synced_container_path(container_file)"},{"line_number":112,"context_line":"        try:"},{"line_number":113,"context_line":"            os.unlink(sync_file)"}],"source_content_type":"text/x-python","patch_set":22,"id":"ba8a016a_277f7a16","line":110,"updated":"2015-11-18 17:27:40.000000000","message":"is there a reason why \n\nremove_synced_container\n\nand \n\n_remove_synced_container\n\nare 2 separate functions?","commit_id":"d955f462a4fdda2223281a472d2bfd100d24081e"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"bc000330a9c91ed211cb9a6639c3a7f13457793d","unresolved":false,"context_lines":[{"line_number":107,"context_line":"        \"\"\""},{"line_number":108,"context_line":"        return self._remove_synced_container(broker.db_file, broker.db_dir)"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    def _remove_synced_container(self, container_file, container_path):"},{"line_number":111,"context_line":"        sync_file \u003d self._container_to_synced_container_path(container_file)"},{"line_number":112,"context_line":"        try:"},{"line_number":113,"context_line":"            os.unlink(sync_file)"}],"source_content_type":"text/x-python","patch_set":22,"id":"ba8a016a_633316ac","line":110,"in_reply_to":"ba8a016a_277f7a16","updated":"2015-11-19 13:38:35.000000000","message":"Historical one, when I was not sure what the API should look like. Fixed that.","commit_id":"d955f462a4fdda2223281a472d2bfd100d24081e"},{"author":{"_account_id":995,"name":"David Goetz","email":"david.goetz@rackspace.com","username":"david-goetz"},"change_message_id":"7e18713d1db533fe9ab8f2799734055600e245c9","unresolved":false,"context_lines":[{"line_number":110,"context_line":"    def _remove_synced_container(self, container_file, container_path):"},{"line_number":111,"context_line":"        sync_file \u003d self._container_to_synced_container_path(container_file)"},{"line_number":112,"context_line":"        try:"},{"line_number":113,"context_line":"            os.unlink(sync_file)"},{"line_number":114,"context_line":"        except OSError as oserr:"},{"line_number":115,"context_line":"            if oserr.errno !\u003d errno.ENOENT:"},{"line_number":116,"context_line":"                raise oserr"}],"source_content_type":"text/x-python","patch_set":22,"id":"ba8a016a_79b0c68a","line":113,"updated":"2015-11-18 17:27:40.000000000","message":"so when I put the lexists in sync.py it gets here but the sync_file path is wrong. I get something like:\n\n/srv/2/node/sdb2/sync_/sync_containers/699/d53/aee564aa0b769e35aca56409c7080d53/aee564aa0b769e35aca56409c7080d53.db","commit_id":"d955f462a4fdda2223281a472d2bfd100d24081e"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"bc000330a9c91ed211cb9a6639c3a7f13457793d","unresolved":false,"context_lines":[{"line_number":110,"context_line":"    def _remove_synced_container(self, container_file, container_path):"},{"line_number":111,"context_line":"        sync_file \u003d self._container_to_synced_container_path(container_file)"},{"line_number":112,"context_line":"        try:"},{"line_number":113,"context_line":"            os.unlink(sync_file)"},{"line_number":114,"context_line":"        except OSError as oserr:"},{"line_number":115,"context_line":"            if oserr.errno !\u003d errno.ENOENT:"},{"line_number":116,"context_line":"                raise oserr"}],"source_content_type":"text/x-python","patch_set":22,"id":"ba8a016a_c3220252","line":113,"in_reply_to":"ba8a016a_79b0c68a","updated":"2015-11-19 13:38:35.000000000","message":"Right, this is the second bug you have found :-)\n(The first being s/exists/lexists)\n\nHere the problem is that I always treat the path as being the .db path and convert to the symlink path. However, we actually have two scenarios:\n1. Getting a symlink - when called from the sync daemon\n2. Getting the real path - when called from the replicator daemon\nCode and tests were fixed accordingly.\nThanks!!!","commit_id":"d955f462a4fdda2223281a472d2bfd100d24081e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"377478c9669b0ab3185759dfb02a536e80425103","unresolved":false,"context_lines":[{"line_number":1,"context_line":""},{"line_number":2,"context_line":"# Copyright (c) 2010-2012 OpenStack Foundation"},{"line_number":3,"context_line":"#"},{"line_number":4,"context_line":"# Licensed under the Apache License, Version 2.0 (the \"License\");"},{"line_number":5,"context_line":"# you may not use this file except in compliance with the License."}],"source_content_type":"text/x-python","patch_set":23,"id":"9a8ffd7b_69f7298d","line":2,"updated":"2015-11-30 16:49:30.000000000","message":"nit: s/2010-2012/2015","commit_id":"fa6a27b075651a6a731715e324121d36f761c083"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"377478c9669b0ab3185759dfb02a536e80425103","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        or more generally:"},{"line_number":48,"context_line":"        devices/device/containers/part/.../*.db"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"        This function splits the path into a list with the following elements:"},{"line_number":51,"context_line":"        (\u0027/srv/node/\u0027, \u0027sdb/\u0027, part/.../*.db)"},{"line_number":52,"context_line":"        or more generally to:"},{"line_number":53,"context_line":"        (devices, device, part/.../*.db)"}],"source_content_type":"text/x-python","patch_set":23,"id":"9a8ffd7b_091c5d76","line":50,"updated":"2015-11-30 16:49:30.000000000","message":"nit: the method returns a tuple not a list","commit_id":"fa6a27b075651a6a731715e324121d36f761c083"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"377478c9669b0ab3185759dfb02a536e80425103","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        \"\"\""},{"line_number":55,"context_line":"        devices \u003d self.devices"},{"line_number":56,"context_line":"        device \u003d path[len(devices):path.find(DATADIR)]"},{"line_number":57,"context_line":"        suffix \u003d path[path.find(DATADIR) + len(DATADIR) + 1:]"},{"line_number":58,"context_line":"        return (devices, device, suffix)"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    def _container_to_synced_container_path(self, path):"}],"source_content_type":"text/x-python","patch_set":23,"id":"9a8ffd7b_fb9f5a63","line":57,"updated":"2015-11-30 16:49:30.000000000","message":"the use of \u0027suffix\u0027 here threw me briefly, because of its specific meaning in swift datadir paths.","commit_id":"fa6a27b075651a6a731715e324121d36f761c083"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"377478c9669b0ab3185759dfb02a536e80425103","unresolved":false,"context_lines":[{"line_number":110,"context_line":"            os.unlink(sync_file)"},{"line_number":111,"context_line":"        except OSError as oserr:"},{"line_number":112,"context_line":"            if oserr.errno !\u003d errno.ENOENT:"},{"line_number":113,"context_line":"                raise oserr"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    def synced_containers_generator(self):"},{"line_number":116,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":23,"id":"9a8ffd7b_3b5402c9","line":113,"updated":"2015-11-30 16:49:30.000000000","message":"I think a call to os.removedirs here would be useful, to clean up any empty dirs. Otherwise when containers cease to be sync\u0027d a growing number of empty leaf nodes accumulate in the sync_store dir.\n\nIn fact I think it could go at line 111 inside the try/except.","commit_id":"fa6a27b075651a6a731715e324121d36f761c083"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"bc36d7693b897bf984f03c9f87f53ac44ff975e1","unresolved":false,"context_lines":[{"line_number":89,"context_line":"                    raise oserr"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        try:"},{"line_number":92,"context_line":"            os.symlink(broker.db_file, sync_file)"},{"line_number":93,"context_line":"        except OSError as oserr:"},{"line_number":94,"context_line":"            if oserr.errno !\u003d errno.EEXIST:"},{"line_number":95,"context_line":"                raise oserr"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a710552_e002da0a","line":92,"updated":"2015-12-16 12:37:07.000000000","message":"needs a symlink to the .pending file too, or maybe symlink to the directory\n\nwithout this the sync daemon will fail to commit pending object updates to the db before sync\u0027ing the container","commit_id":"b57a7a810d7fd635d7f2107d147ee0ab72e01216"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7a10ba36645654a9bc283d36870616a28d09361d","unresolved":false,"context_lines":[{"line_number":122,"context_line":"                       replicated\u0027"},{"line_number":123,"context_line":"        \"\"\""},{"line_number":124,"context_line":"        if \u0027X-Container-Sync-To\u0027 not in broker.metadata:"},{"line_number":125,"context_line":"            return"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        sync_to \u003d broker.metadata[\u0027X-Container-Sync-To\u0027][0]"},{"line_number":128,"context_line":"        if broker.is_deleted() or sync_to \u003d\u003d \u0027\u0027:"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a710552_2e2a437f","line":125,"updated":"2015-12-15 17:38:10.000000000","message":"note to self: if broker metadata does not x-container-sync-to then it has *never* been set. If it has been \u0027deleted\u0027 then broker will have x-container-sync-key \u003d \u0027\u0027","commit_id":"b57a7a810d7fd635d7f2107d147ee0ab72e01216"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"399151b9656704a7e2ee6329e40bd896a2cc4b23","unresolved":false,"context_lines":[{"line_number":122,"context_line":"                       replicated\u0027"},{"line_number":123,"context_line":"        \"\"\""},{"line_number":124,"context_line":"        if \u0027X-Container-Sync-To\u0027 not in broker.metadata:"},{"line_number":125,"context_line":"            return"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        sync_to \u003d broker.metadata[\u0027X-Container-Sync-To\u0027][0]"},{"line_number":128,"context_line":"        if broker.is_deleted() or sync_to \u003d\u003d \u0027\u0027:"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a710552_c93b5d9b","line":125,"in_reply_to":"5a710552_2e2a437f","updated":"2015-12-16 06:44:37.000000000","message":"Added a comment along these lines","commit_id":"b57a7a810d7fd635d7f2107d147ee0ab72e01216"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7a10ba36645654a9bc283d36870616a28d09361d","unresolved":false,"context_lines":[{"line_number":145,"context_line":"        yielding the path of the container db"},{"line_number":146,"context_line":"        \"\"\""},{"line_number":147,"context_line":"        all_locs \u003d audit_location_generator(self.devices, SYNC_DATADIR, \u0027.db\u0027,"},{"line_number":148,"context_line":"                                            mount_check\u003dFalse,"},{"line_number":149,"context_line":"                                            logger\u003dself.logger)"},{"line_number":150,"context_line":"        for path, device, partition in all_locs:"},{"line_number":151,"context_line":"            yield path"}],"source_content_type":"text/x-python","patch_set":26,"id":"7a740942_cfc67999","line":148,"updated":"2015-12-15 17:38:10.000000000","message":"on master this call (which was made from sync.py) passes whatever value is configured for mount_check, here you have forced it to false. Perhaps configured mount_check value should be passed to the sync_store constructor and used here?","commit_id":"b57a7a810d7fd635d7f2107d147ee0ab72e01216"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"399151b9656704a7e2ee6329e40bd896a2cc4b23","unresolved":false,"context_lines":[{"line_number":145,"context_line":"        yielding the path of the container db"},{"line_number":146,"context_line":"        \"\"\""},{"line_number":147,"context_line":"        all_locs \u003d audit_location_generator(self.devices, SYNC_DATADIR, \u0027.db\u0027,"},{"line_number":148,"context_line":"                                            mount_check\u003dFalse,"},{"line_number":149,"context_line":"                                            logger\u003dself.logger)"},{"line_number":150,"context_line":"        for path, device, partition in all_locs:"},{"line_number":151,"context_line":"            yield path"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a710552_18eb04b2","line":148,"in_reply_to":"7a740942_cfc67999","updated":"2015-12-16 06:44:37.000000000","message":"Done. I believe it was inherited from the original container sync code, but I do not see a reason why not to add this.","commit_id":"b57a7a810d7fd635d7f2107d147ee0ab72e01216"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d3272fdaa3aa986b2e5ab3997a548cf06ecba54f","unresolved":false,"context_lines":[{"line_number":38,"context_line":"            devices \u003d \u0027/%s\u0027 % devices"},{"line_number":39,"context_line":"        if not devices.endswith(\u0027/\u0027):"},{"line_number":40,"context_line":"            devices \u003d \u0027%s/\u0027 % devices"},{"line_number":41,"context_line":"        self.devices \u003d devices"},{"line_number":42,"context_line":"        self.mount_check \u003d mount_check"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    def _split_container_dir_path(self, path):"}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_970f7c87","line":41,"updated":"2015-12-18 19:01:55.000000000","message":"You could use:\n\n   os.path.normpath(os.path.join(\u0027/\u0027, devices))+os.sep\n\nwhich takes care of platform dependent paths and any relative path parts. Having said that, I\u0027m sure there are places elsewhere in code where it\u0027s not so careful.","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"133e8a97d28c0e1b328e9f7ade6cfdeeae57ae8e","unresolved":false,"context_lines":[{"line_number":38,"context_line":"            devices \u003d \u0027/%s\u0027 % devices"},{"line_number":39,"context_line":"        if not devices.endswith(\u0027/\u0027):"},{"line_number":40,"context_line":"            devices \u003d \u0027%s/\u0027 % devices"},{"line_number":41,"context_line":"        self.devices \u003d devices"},{"line_number":42,"context_line":"        self.mount_check \u003d mount_check"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    def _split_container_dir_path(self, path):"}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_35b55d30","line":41,"in_reply_to":"3a7e1126_970f7c87","updated":"2015-12-19 00:03:21.000000000","message":"Done. It is both less lines of code, and more robust. Nice.","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d3272fdaa3aa986b2e5ab3997a548cf06ecba54f","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        self.devices \u003d devices"},{"line_number":42,"context_line":"        self.mount_check \u003d mount_check"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    def _split_container_dir_path(self, path):"},{"line_number":45,"context_line":"        \"\"\""},{"line_number":46,"context_line":"        path is assumed to be of the form:"},{"line_number":47,"context_line":"        /srv/node/sdb/containers/part/.../*.db"}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_bd8059c1","line":44,"updated":"2015-12-18 19:01:55.000000000","message":"this method could be merged into _container_to_synced_container_path - that is the only call site","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"133e8a97d28c0e1b328e9f7ade6cfdeeae57ae8e","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        self.devices \u003d devices"},{"line_number":42,"context_line":"        self.mount_check \u003d mount_check"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    def _split_container_dir_path(self, path):"},{"line_number":45,"context_line":"        \"\"\""},{"line_number":46,"context_line":"        path is assumed to be of the form:"},{"line_number":47,"context_line":"        /srv/node/sdb/containers/part/.../*.db"}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_55d10922","line":44,"in_reply_to":"3a7e1126_bd8059c1","updated":"2015-12-19 00:03:21.000000000","message":"Yep. Done.","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d3272fdaa3aa986b2e5ab3997a548cf06ecba54f","unresolved":false,"context_lines":[{"line_number":46,"context_line":"        path is assumed to be of the form:"},{"line_number":47,"context_line":"        /srv/node/sdb/containers/part/.../*.db"},{"line_number":48,"context_line":"        or more generally:"},{"line_number":49,"context_line":"        devices/device/containers/part/.../*.db"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"        This function splits the path into a tuple with the following elements:"},{"line_number":52,"context_line":"        (\u0027/srv/node/\u0027, \u0027sdb/\u0027, part/.../*.db)"}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_3d5ea944","line":49,"updated":"2015-12-18 19:01:55.000000000","message":"what if device\u003d\u0027containers\u0027? i.e. path\u003d/srv/node/containers/containers. IDK if swift can cope with that in general???\n\nThe alternative is to assume a fixed number of path parts and split on \u0027/\u0027.\n\nI feel there ought to be a pattern for this somewhere in swift codebase, but I\u0027m not sure where ;)","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"133e8a97d28c0e1b328e9f7ade6cfdeeae57ae8e","unresolved":false,"context_lines":[{"line_number":46,"context_line":"        path is assumed to be of the form:"},{"line_number":47,"context_line":"        /srv/node/sdb/containers/part/.../*.db"},{"line_number":48,"context_line":"        or more generally:"},{"line_number":49,"context_line":"        devices/device/containers/part/.../*.db"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"        This function splits the path into a tuple with the following elements:"},{"line_number":52,"context_line":"        (\u0027/srv/node/\u0027, \u0027sdb/\u0027, part/.../*.db)"}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_4975f461","line":49,"in_reply_to":"3a7e1126_3d5ea944","updated":"2015-12-19 00:03:21.000000000","message":"I believe that switching s/find/rfind below resolves this. Making that change.","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d3272fdaa3aa986b2e5ab3997a548cf06ecba54f","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        (devices, device, part/.../*.db)"},{"line_number":55,"context_line":"        \"\"\""},{"line_number":56,"context_line":"        devices \u003d self.devices"},{"line_number":57,"context_line":"        device \u003d path[len(devices):path.find(DATADIR)]"},{"line_number":58,"context_line":"        rest \u003d path[path.find(DATADIR) + len(DATADIR) + 1:]"},{"line_number":59,"context_line":"        return (devices, device, rest)"},{"line_number":60,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_975d3c7b","line":57,"updated":"2015-12-18 19:01:55.000000000","message":"to be really safe, do we need \n\n  path \u003d os.path.normpath(path)\n\nbefore slicing the path? In case there are any \u0027..\u0027 or \u0027//\u0027 in path?\n\nIts almost certainly safe, given where path comes from, I am just being paranoid.","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"133e8a97d28c0e1b328e9f7ade6cfdeeae57ae8e","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        (devices, device, part/.../*.db)"},{"line_number":55,"context_line":"        \"\"\""},{"line_number":56,"context_line":"        devices \u003d self.devices"},{"line_number":57,"context_line":"        device \u003d path[len(devices):path.find(DATADIR)]"},{"line_number":58,"context_line":"        rest \u003d path[path.find(DATADIR) + len(DATADIR) + 1:]"},{"line_number":59,"context_line":"        return (devices, device, rest)"},{"line_number":60,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_c9d5641c","line":57,"in_reply_to":"3a7e1126_975d3c7b","updated":"2015-12-19 00:03:21.000000000","message":"yep. Done","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d3272fdaa3aa986b2e5ab3997a548cf06ecba54f","unresolved":false,"context_lines":[{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def _container_to_synced_container_path(self, path):"},{"line_number":62,"context_line":"        spath \u003d self._split_container_dir_path(path)"},{"line_number":63,"context_line":"        return os.path.join(spath[0], spath[1], SYNC_DATADIR, spath[2])"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def add_synced_container(self, broker):"},{"line_number":66,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_3db0e989","line":63,"updated":"2015-12-18 19:01:55.000000000","message":"Is the result the same as path.replace(DATADIR, SYNC_DATADIR, 1)?","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"133e8a97d28c0e1b328e9f7ade6cfdeeae57ae8e","unresolved":false,"context_lines":[{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def _container_to_synced_container_path(self, path):"},{"line_number":62,"context_line":"        spath \u003d self._split_container_dir_path(path)"},{"line_number":63,"context_line":"        return os.path.join(spath[0], spath[1], SYNC_DATADIR, spath[2])"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def add_synced_container(self, broker):"},{"line_number":66,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_49d17422","line":63,"in_reply_to":"3a7e1126_3db0e989","updated":"2015-12-19 00:03:21.000000000","message":"I guess that you are right. However, once changing the code to accommodate path\u003d/srv/node/containers/containers - which I hope I did - this is no longer the case","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d3272fdaa3aa986b2e5ab3997a548cf06ecba54f","unresolved":false,"context_lines":[{"line_number":82,"context_line":"            return"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        sync_path \u003d os.path.dirname(sync_file)"},{"line_number":85,"context_line":"        if not os.path.isdir(sync_path):"},{"line_number":86,"context_line":"            try:"},{"line_number":87,"context_line":"                os.makedirs(sync_path)"},{"line_number":88,"context_line":"            except OSError as oserr:"}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_c8548500","line":85,"updated":"2015-12-18 19:01:55.000000000","message":"see swift/common/utils.py:895 - existing utility function to do this mkdir","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"133e8a97d28c0e1b328e9f7ade6cfdeeae57ae8e","unresolved":false,"context_lines":[{"line_number":82,"context_line":"            return"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        sync_path \u003d os.path.dirname(sync_file)"},{"line_number":85,"context_line":"        if not os.path.isdir(sync_path):"},{"line_number":86,"context_line":"            try:"},{"line_number":87,"context_line":"                os.makedirs(sync_path)"},{"line_number":88,"context_line":"            except OSError as oserr:"}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_e4540500","line":85,"in_reply_to":"3a7e1126_c8548500","updated":"2015-12-19 00:03:21.000000000","message":"yep, replaced","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d3272fdaa3aa986b2e5ab3997a548cf06ecba54f","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        try:"},{"line_number":93,"context_line":"            os.symlink(broker.db_file, sync_file)"},{"line_number":94,"context_line":"        except OSError as oserr:"},{"line_number":95,"context_line":"            if oserr.errno !\u003d errno.EEXIST:"},{"line_number":96,"context_line":"                raise oserr"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    def remove_synced_container(self, broker):"}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_486895b9","line":95,"updated":"2015-12-18 19:01:55.000000000","message":"if oserr.errno !\u003d errno.EEXIST or not os.path.islink(sync_file):","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"133e8a97d28c0e1b328e9f7ade6cfdeeae57ae8e","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        try:"},{"line_number":93,"context_line":"            os.symlink(broker.db_file, sync_file)"},{"line_number":94,"context_line":"        except OSError as oserr:"},{"line_number":95,"context_line":"            if oserr.errno !\u003d errno.EEXIST:"},{"line_number":96,"context_line":"                raise oserr"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    def remove_synced_container(self, broker):"}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_845501fb","line":95,"in_reply_to":"3a7e1126_486895b9","updated":"2015-12-19 00:03:21.000000000","message":"Done","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d3272fdaa3aa986b2e5ab3997a548cf06ecba54f","unresolved":false,"context_lines":[{"line_number":102,"context_line":""},{"line_number":103,"context_line":"        :param broker: An instance of ContainerBroker representing the"},{"line_number":104,"context_line":"                       container to remove."},{"line_number":105,"context_line":"        :param link: Indicates whether the broker db_file is a symlink"},{"line_number":106,"context_line":"        \"\"\""},{"line_number":107,"context_line":"        sync_file \u003d broker.db_file"},{"line_number":108,"context_line":"        sync_file \u003d self._container_to_synced_container_path(sync_file)"}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_22052845","line":105,"updated":"2015-12-18 19:01:55.000000000","message":"remove param link","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"133e8a97d28c0e1b328e9f7ade6cfdeeae57ae8e","unresolved":false,"context_lines":[{"line_number":102,"context_line":""},{"line_number":103,"context_line":"        :param broker: An instance of ContainerBroker representing the"},{"line_number":104,"context_line":"                       container to remove."},{"line_number":105,"context_line":"        :param link: Indicates whether the broker db_file is a symlink"},{"line_number":106,"context_line":"        \"\"\""},{"line_number":107,"context_line":"        sync_file \u003d broker.db_file"},{"line_number":108,"context_line":"        sync_file \u003d self._container_to_synced_container_path(sync_file)"}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_49a654af","line":105,"in_reply_to":"3a7e1126_22052845","updated":"2015-12-19 00:03:21.000000000","message":"Done.","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d3272fdaa3aa986b2e5ab3997a548cf06ecba54f","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        according to the broker\u0027s metadata."},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"        :param broker: An instance of ContainerBroker being \u0027post"},{"line_number":122,"context_line":"                       replicated\u0027"},{"line_number":123,"context_line":"        \"\"\""},{"line_number":124,"context_line":"        # If the broker metadata does not have x-container-sync-to then"},{"line_number":125,"context_line":"        # it has *never* been set. Make sure we do nothing in this case"}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_d738a4f1","line":122,"updated":"2015-12-18 19:01:55.000000000","message":"remove \u0027being post replicated\u0027 - its is also called from the server now","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"133e8a97d28c0e1b328e9f7ade6cfdeeae57ae8e","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        according to the broker\u0027s metadata."},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"        :param broker: An instance of ContainerBroker being \u0027post"},{"line_number":122,"context_line":"                       replicated\u0027"},{"line_number":123,"context_line":"        \"\"\""},{"line_number":124,"context_line":"        # If the broker metadata does not have x-container-sync-to then"},{"line_number":125,"context_line":"        # it has *never* been set. Make sure we do nothing in this case"}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_c9dfa40b","line":122,"in_reply_to":"3a7e1126_d738a4f1","updated":"2015-12-19 00:03:21.000000000","message":"Done","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d3272fdaa3aa986b2e5ab3997a548cf06ecba54f","unresolved":false,"context_lines":[{"line_number":131,"context_line":"            self.remove_synced_container(broker)"},{"line_number":132,"context_line":"            return"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"        if \u0027X-Container-Sync-Key\u0027 not in broker.metadata:"},{"line_number":135,"context_line":"            return"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"        sync_key \u003d broker.metadata[\u0027X-Container-Sync-Key\u0027][0]"}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_02682c86","line":134,"updated":"2015-12-18 19:01:55.000000000","message":"hmmm, do we want to also remove the symlink if sync-key is not set?","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"133e8a97d28c0e1b328e9f7ade6cfdeeae57ae8e","unresolved":false,"context_lines":[{"line_number":131,"context_line":"            self.remove_synced_container(broker)"},{"line_number":132,"context_line":"            return"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"        if \u0027X-Container-Sync-Key\u0027 not in broker.metadata:"},{"line_number":135,"context_line":"            return"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"        sync_key \u003d broker.metadata[\u0027X-Container-Sync-Key\u0027][0]"}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_7f1e5e60","line":134,"in_reply_to":"3a7e1126_02682c86","updated":"2015-12-19 00:03:21.000000000","message":"Yes we do. I got confused as to the case where we do not want to do anything, which is both items do not appear in the broker\u0027s metadata. I have restructured the function to make more sense, adding exhaustive comments while at it.","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d3272fdaa3aa986b2e5ab3997a548cf06ecba54f","unresolved":false,"context_lines":[{"line_number":135,"context_line":"            return"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"        sync_key \u003d broker.metadata[\u0027X-Container-Sync-Key\u0027][0]"},{"line_number":138,"context_line":"        if sync_key \u003d\u003d \u0027\u0027:"},{"line_number":139,"context_line":"            self.remove_synced_container(broker)"},{"line_number":140,"context_line":"            return"},{"line_number":141,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_a2a358b1","line":138,"updated":"2015-12-18 19:01:55.000000000","message":"Can this be moved into the previous \u0027if\u0027, e.g.:\n\n  if broker.is_deleted() or sync_to \u003d\u003d \u0027\u0027 or sync_key\u003d\u003d\u0027\u0027:\n\nor did i miss the significance of lines 134-135??","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"133e8a97d28c0e1b328e9f7ade6cfdeeae57ae8e","unresolved":false,"context_lines":[{"line_number":135,"context_line":"            return"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"        sync_key \u003d broker.metadata[\u0027X-Container-Sync-Key\u0027][0]"},{"line_number":138,"context_line":"        if sync_key \u003d\u003d \u0027\u0027:"},{"line_number":139,"context_line":"            self.remove_synced_container(broker)"},{"line_number":140,"context_line":"            return"},{"line_number":141,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_ff324ef1","line":138,"in_reply_to":"3a7e1126_a2a358b1","updated":"2015-12-19 00:03:21.000000000","message":"You haven\u0027t missed that. Restructured the function.","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d3272fdaa3aa986b2e5ab3997a548cf06ecba54f","unresolved":false,"context_lines":[{"line_number":154,"context_line":"            # initiating a container broker. The broker would break if not"},{"line_number":155,"context_line":"            # given the db real path, as it e.g. assumes the existence of"},{"line_number":156,"context_line":"            # .pending in the same path"},{"line_number":157,"context_line":"            yield os.path.realpath(path)"}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_7d9c315d","line":157,"updated":"2015-12-18 19:01:55.000000000","message":"It\u0027s a shame that realpath, I guess, will need to load all the inodes for the absolute path to the containers/ subdir, possibly meaning more disk access. But I haven\u0027t yet thought of a way to avoid that, short of huge change to the way broker is instantiated.\n\nUnless (crazy idea...) instead of a symlink you stored an empty file whose name represented the absolute path to the db file (i.e.same as the result of realpath(), but with / replaced with . or some other encoding) - so the path yielded by all_locs has a filename that you parse (replace . with /) and yield to the sync daemon.\n\n^^ That could be a complete unnecessary red-herring, just mentioning it while I think of it. The cost of the realpath() call is only going to be seen when inodes are being evicted from buffer cache, so a loaded cluster.","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"133e8a97d28c0e1b328e9f7ade6cfdeeae57ae8e","unresolved":false,"context_lines":[{"line_number":154,"context_line":"            # initiating a container broker. The broker would break if not"},{"line_number":155,"context_line":"            # given the db real path, as it e.g. assumes the existence of"},{"line_number":156,"context_line":"            # .pending in the same path"},{"line_number":157,"context_line":"            yield os.path.realpath(path)"}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_c97b24f4","line":157,"in_reply_to":"3a7e1126_7d9c315d","updated":"2015-12-19 00:03:21.000000000","message":"Well, another option is to do an inverse translation from synced_containrs path to container path - which is pretty straight forward with the current symlink structure. If this changes we can go eat some herrings.","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":46,"context_line":"        # devices, device, rest"},{"line_number":47,"context_line":"        devices \u003d self.devices"},{"line_number":48,"context_line":"        path \u003d os.path.normpath(path)"},{"line_number":49,"context_line":"        device \u003d path[len(devices):path.rfind(DATADIR)]"},{"line_number":50,"context_line":"        rest \u003d path[path.rfind(DATADIR) + len(DATADIR) + 1:]"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        return os.path.join(devices, device, SYNC_DATADIR, rest)"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_4e1f0f08","line":49,"updated":"2015-12-21 17:01:17.000000000","message":"yes, rfind!","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":91,"context_line":"            os.symlink(broker.db_file, sync_file)"},{"line_number":92,"context_line":"        except OSError as oserr:"},{"line_number":93,"context_line":"            if oserr.errno !\u003d errno.EEXIST or\\"},{"line_number":94,"context_line":"               not os.path.islink(sync_file):"},{"line_number":95,"context_line":"                    raise oserr"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def remove_synced_container(self, broker):"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_4ed06ff6","line":94,"updated":"2015-12-21 17:01:17.000000000","message":"nit: use parentheses to avoid the \\ line break\n\n  if (oserr.errno !\u003d errno.EEXIST or\n          not os.path.islink(sync_file)):","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":91,"context_line":"            os.symlink(broker.db_file, sync_file)"},{"line_number":92,"context_line":"        except OSError as oserr:"},{"line_number":93,"context_line":"            if oserr.errno !\u003d errno.EEXIST or\\"},{"line_number":94,"context_line":"               not os.path.islink(sync_file):"},{"line_number":95,"context_line":"                    raise oserr"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def remove_synced_container(self, broker):"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_9a72abe4","line":94,"in_reply_to":"3a7e1126_4ed06ff6","updated":"2016-01-01 21:44:24.000000000","message":"Good to know :-)\nDone.","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":116,"context_line":"        Add or remove a symlink to/from the sync-containers directory"},{"line_number":117,"context_line":"        according to the broker\u0027s metadata."},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"        :param broker: An instance of ContainerBroker"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"        Decide according to the broker x-container-sync-to and"},{"line_number":122,"context_line":"        x-container-sync-key whether a symlink needs to be added or"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_5b710c75","line":119,"updated":"2015-12-21 17:01:17.000000000","message":"I think it is convention (although apparently not enforced [1]) to have the param definitions at the end of the docstring.\n\n[1] http://docs.openstack.org/developer/hacking/#docstrings","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":116,"context_line":"        Add or remove a symlink to/from the sync-containers directory"},{"line_number":117,"context_line":"        according to the broker\u0027s metadata."},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"        :param broker: An instance of ContainerBroker"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"        Decide according to the broker x-container-sync-to and"},{"line_number":122,"context_line":"        x-container-sync-key whether a symlink needs to be added or"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_9a494b8e","line":119,"in_reply_to":"3a7e1126_5b710c75","updated":"2016-01-01 21:44:24.000000000","message":"yeah. probably the result of adding the func description at a later stage...","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":125,"context_line":"        We mention that if both metadata items do not appear"},{"line_number":126,"context_line":"        at all, the container has never been set for sync in which case"},{"line_number":127,"context_line":"        we do nothing. This is important as this method is called"},{"line_number":128,"context_line":"        for ALL containers from the container replicator."},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"        Once we realize that we do need to do something, we check if"},{"line_number":131,"context_line":"        the container is marked for delete, in which case we want to"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_1b1b84cb","line":128,"updated":"2015-12-21 17:01:17.000000000","message":"I think this was my mistake on an earlier review - the container may have had sync metadata before, but not since (now - reclaim_age) - older metadata items do get cleaned up eventually:\n\nsee swift.common.db.DatabaseBroker#_reclaim\n\nSo more precisely:\n\n  if both metadata items do not appear at all, the container\n  has not been set for sync in reclaim_age, in which case...","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":125,"context_line":"        We mention that if both metadata items do not appear"},{"line_number":126,"context_line":"        at all, the container has never been set for sync in which case"},{"line_number":127,"context_line":"        we do nothing. This is important as this method is called"},{"line_number":128,"context_line":"        for ALL containers from the container replicator."},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"        Once we realize that we do need to do something, we check if"},{"line_number":131,"context_line":"        the container is marked for delete, in which case we want to"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_7a3b37ec","line":128,"in_reply_to":"3a7e1126_1b1b84cb","updated":"2016-01-01 21:44:24.000000000","message":"right, fixed.","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":132,"context_line":"        remove the symlink"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"        For adding a symlink we notice that both x-container-sync-to and"},{"line_number":135,"context_line":"        x-container-sync-key exist and valid, that is, are not empty."},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"        At this point we know we need to do something, the container"},{"line_number":138,"context_line":"        is not marked for delete and the condition to add a symlink"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_9b5c547e","line":135,"updated":"2015-12-21 17:01:17.000000000","message":"nit: \u0027exist and are valid\u0027","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":132,"context_line":"        remove the symlink"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"        For adding a symlink we notice that both x-container-sync-to and"},{"line_number":135,"context_line":"        x-container-sync-key exist and valid, that is, are not empty."},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"        At this point we know we need to do something, the container"},{"line_number":138,"context_line":"        is not marked for delete and the condition to add a symlink"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_3a261fcf","line":135,"in_reply_to":"3a7e1126_9b5c547e","updated":"2016-01-01 21:44:24.000000000","message":"One day I will write proper English. Thanks!","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        sync_key \u003d \u0027\u0027"},{"line_number":158,"context_line":"        if \u0027X-Container-Sync-Key\u0027 in broker.metadata:"},{"line_number":159,"context_line":"            sync_key \u003d broker.metadata[\u0027X-Container-Sync-Key\u0027][0]"},{"line_number":160,"context_line":"        if sync_to !\u003d \u0027\u0027 and sync_key !\u003d \u0027\u0027:"},{"line_number":161,"context_line":"            self.add_synced_container(broker)"},{"line_number":162,"context_line":"            return"},{"line_number":163,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_1b8b8469","line":160,"updated":"2015-12-21 17:01:17.000000000","message":"nit: this could be \n\n  if sync_to and sync_key:\n\nwith the advantage that if someone were ever to change the broker so that it\u0027s metadata could contain sync_to \u003d\u003d None (instead if \u0027\u0027) then this code would still work.","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        sync_key \u003d \u0027\u0027"},{"line_number":158,"context_line":"        if \u0027X-Container-Sync-Key\u0027 in broker.metadata:"},{"line_number":159,"context_line":"            sync_key \u003d broker.metadata[\u0027X-Container-Sync-Key\u0027][0]"},{"line_number":160,"context_line":"        if sync_to !\u003d \u0027\u0027 and sync_key !\u003d \u0027\u0027:"},{"line_number":161,"context_line":"            self.add_synced_container(broker)"},{"line_number":162,"context_line":"            return"},{"line_number":163,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_fa06471f","line":160,"in_reply_to":"3a7e1126_1b8b8469","updated":"2016-01-01 21:44:24.000000000","message":"Not to mention that we reduce the code :-)","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":177,"context_line":"            # given the db real path, as it e.g. assumes the existence of"},{"line_number":178,"context_line":"            # .pending in the same path"},{"line_number":179,"context_line":"            path \u003d self._synced_container_to_container_path(path)"},{"line_number":180,"context_line":"            yield path"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_bb66f027","line":180,"updated":"2015-12-21 17:01:17.000000000","message":"nit: could just be \n\n  yield self._synced_container_to_container_path(path)","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":177,"context_line":"            # given the db real path, as it e.g. assumes the existence of"},{"line_number":178,"context_line":"            # .pending in the same path"},{"line_number":179,"context_line":"            path \u003d self._synced_container_to_container_path(path)"},{"line_number":180,"context_line":"            yield path"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_5d118d51","line":180,"in_reply_to":"3a7e1126_bb66f027","updated":"2016-01-01 21:44:24.000000000","message":"I guess that those pep8 errors made me hysteric w.r.t. line length. Again, I always vote for less LOC.","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"8fc9ba0594a71c374901dc3602e032edb381a5fb","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    listed."},{"line_number":33,"context_line":"    \"\"\""},{"line_number":34,"context_line":"    def __init__(self, devices, logger, mount_check):"},{"line_number":35,"context_line":"        self.devices \u003d os.path.normpath(os.path.join(\u0027/\u0027, devices)) + os.sep"},{"line_number":36,"context_line":"        self.logger \u003d logger"},{"line_number":37,"context_line":"        self.mount_check \u003d mount_check"},{"line_number":38,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_70b3fde9","line":35,"updated":"2016-01-05 06:04:31.000000000","message":"Not 100% sure about this, your going to the effort of using os.sep yet your adding a \u0027/\u0027 to the front of devices.","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"1894a6f77a2cee4c903ef90cbea78e7e8b593469","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    listed."},{"line_number":33,"context_line":"    \"\"\""},{"line_number":34,"context_line":"    def __init__(self, devices, logger, mount_check):"},{"line_number":35,"context_line":"        self.devices \u003d os.path.normpath(os.path.join(\u0027/\u0027, devices)) + os.sep"},{"line_number":36,"context_line":"        self.logger \u003d logger"},{"line_number":37,"context_line":"        self.mount_check \u003d mount_check"},{"line_number":38,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_0c8fec2a","line":35,"in_reply_to":"fa69d971_70b3fde9","updated":"2016-01-05 21:23:33.000000000","message":"yep. changed \u0027/\u0027 to os.sep\nHowever, I guess that to really make this supported on windows something else is needed. e.g. the drive letter?","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4b85b8ceb32be975e46a15dbe0325c352af16e1e","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        # and x-container-sync-key it has *never* been set. Make sure"},{"line_number":142,"context_line":"        # we do nothing in this case"},{"line_number":143,"context_line":"        if \u0027X-Container-Sync-To\u0027 not in broker.metadata and\\"},{"line_number":144,"context_line":"           \u0027X-Container-Sync-Key\u0027 not in broker.metadata:"},{"line_number":145,"context_line":"            return"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"        if broker.is_deleted():"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_76830e57","line":144,"updated":"2016-01-05 16:58:35.000000000","message":"nit: could use parentheses rather than line break","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"1894a6f77a2cee4c903ef90cbea78e7e8b593469","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        # and x-container-sync-key it has *never* been set. Make sure"},{"line_number":142,"context_line":"        # we do nothing in this case"},{"line_number":143,"context_line":"        if \u0027X-Container-Sync-To\u0027 not in broker.metadata and\\"},{"line_number":144,"context_line":"           \u0027X-Container-Sync-Key\u0027 not in broker.metadata:"},{"line_number":145,"context_line":"            return"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"        if broker.is_deleted():"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_ccb234e5","line":144,"in_reply_to":"fa69d971_76830e57","updated":"2016-01-05 21:23:33.000000000","message":"done.","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4b85b8ceb32be975e46a15dbe0325c352af16e1e","unresolved":false,"context_lines":[{"line_number":150,"context_line":""},{"line_number":151,"context_line":"        # If both x-container-sync-to and x-container-sync-key"},{"line_number":152,"context_line":"        # exist and valid, add the symlink"},{"line_number":153,"context_line":"        sync_to \u003d \u0027\u0027"},{"line_number":154,"context_line":"        if \u0027X-Container-Sync-To\u0027 in broker.metadata:"},{"line_number":155,"context_line":"            sync_to \u003d broker.metadata[\u0027X-Container-Sync-To\u0027][0]"},{"line_number":156,"context_line":"        sync_key \u003d \u0027\u0027"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_16c17280","line":153,"updated":"2016-01-05 16:58:35.000000000","message":"nit:\n\n  sync_to \u003d sync_key \u003d None","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"1894a6f77a2cee4c903ef90cbea78e7e8b593469","unresolved":false,"context_lines":[{"line_number":150,"context_line":""},{"line_number":151,"context_line":"        # If both x-container-sync-to and x-container-sync-key"},{"line_number":152,"context_line":"        # exist and valid, add the symlink"},{"line_number":153,"context_line":"        sync_to \u003d \u0027\u0027"},{"line_number":154,"context_line":"        if \u0027X-Container-Sync-To\u0027 in broker.metadata:"},{"line_number":155,"context_line":"            sync_to \u003d broker.metadata[\u0027X-Container-Sync-To\u0027][0]"},{"line_number":156,"context_line":"        sync_key \u003d \u0027\u0027"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_6ccc686a","line":153,"in_reply_to":"fa69d971_16c17280","updated":"2016-01-05 21:23:33.000000000","message":"Done","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4b85b8ceb32be975e46a15dbe0325c352af16e1e","unresolved":false,"context_lines":[{"line_number":153,"context_line":"        sync_to \u003d \u0027\u0027"},{"line_number":154,"context_line":"        if \u0027X-Container-Sync-To\u0027 in broker.metadata:"},{"line_number":155,"context_line":"            sync_to \u003d broker.metadata[\u0027X-Container-Sync-To\u0027][0]"},{"line_number":156,"context_line":"        sync_key \u003d \u0027\u0027"},{"line_number":157,"context_line":"        if \u0027X-Container-Sync-Key\u0027 in broker.metadata:"},{"line_number":158,"context_line":"            sync_key \u003d broker.metadata[\u0027X-Container-Sync-Key\u0027][0]"},{"line_number":159,"context_line":"        if sync_to and sync_key:"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_f6737e7e","line":156,"updated":"2016-01-05 16:58:35.000000000","message":"nit: not needed with change above","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"1894a6f77a2cee4c903ef90cbea78e7e8b593469","unresolved":false,"context_lines":[{"line_number":153,"context_line":"        sync_to \u003d \u0027\u0027"},{"line_number":154,"context_line":"        if \u0027X-Container-Sync-To\u0027 in broker.metadata:"},{"line_number":155,"context_line":"            sync_to \u003d broker.metadata[\u0027X-Container-Sync-To\u0027][0]"},{"line_number":156,"context_line":"        sync_key \u003d \u0027\u0027"},{"line_number":157,"context_line":"        if \u0027X-Container-Sync-Key\u0027 in broker.metadata:"},{"line_number":158,"context_line":"            sync_key \u003d broker.metadata[\u0027X-Container-Sync-Key\u0027][0]"},{"line_number":159,"context_line":"        if sync_to and sync_key:"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_4cbf24aa","line":156,"in_reply_to":"fa69d971_f6737e7e","updated":"2016-01-05 21:23:33.000000000","message":"Done","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"6a5742867bca1c90231e38d703e9eb5e21876b34","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    listed."},{"line_number":33,"context_line":"    \"\"\""},{"line_number":34,"context_line":"    def __init__(self, devices, logger, mount_check):"},{"line_number":35,"context_line":"        self.devices \u003d os.path.normpath(os.path.join(os.sep, devices)) + os.sep"},{"line_number":36,"context_line":"        self.logger \u003d logger"},{"line_number":37,"context_line":"        self.mount_check \u003d mount_check"},{"line_number":38,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"fa69d971_617543f8","line":35,"updated":"2016-01-06 00:28:28.000000000","message":"I agree, this wont take account windows C: etc. It was more of the fact that you were forcing a \u0027/\u0027 and then using os.sep in the last patchset, so didn\u0027t see the point :)","commit_id":"2072293e356e4f7122b07e7093f260a6f2b710de"}],"test/probe/test_container_sync.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a20455f7416ea5176823547eff8b278eeaa5e1d1","unresolved":false,"context_lines":[{"line_number":83,"context_line":"        # cycle container replicator to make sure"},{"line_number":84,"context_line":"        # the sync_store gets populated"},{"line_number":85,"context_line":"        Manager([\u0027container-replicator\u0027]).once()"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"        # cycle container-sync"},{"line_number":88,"context_line":"        Manager([\u0027container-sync\u0027]).once()"},{"line_number":89,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_b4c85b56","line":86,"updated":"2015-08-18 19:15:04.000000000","message":"sure, makes sense","commit_id":"aefd20882642662b4f7deeb19ddfa9617cc8c2ff"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7a10ba36645654a9bc283d36870616a28d09361d","unresolved":false,"context_lines":[{"line_number":82,"context_line":""},{"line_number":83,"context_line":"        # Allow Swift to commit container stale changes"},{"line_number":84,"context_line":"        headers, body \u003d client.get_container(self.url, self.token,"},{"line_number":85,"context_line":"                                             source_container)"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"        # cycle container-sync"},{"line_number":88,"context_line":"        # We do this twice ot make sure we go through both"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a710552_0ea2e7ae","line":85,"updated":"2015-12-15 17:38:10.000000000","message":"not sure why this change is required?\n\nBefore sync\u0027in a container db, the sync daemon calls broker.get_info() which commits any entries from pending file into the db.\n\nSee: swift/container/backend.py:399","commit_id":"b57a7a810d7fd635d7f2107d147ee0ab72e01216"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"399151b9656704a7e2ee6329e40bd896a2cc4b23","unresolved":false,"context_lines":[{"line_number":82,"context_line":""},{"line_number":83,"context_line":"        # Allow Swift to commit container stale changes"},{"line_number":84,"context_line":"        headers, body \u003d client.get_container(self.url, self.token,"},{"line_number":85,"context_line":"                                             source_container)"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"        # cycle container-sync"},{"line_number":88,"context_line":"        # We do this twice ot make sure we go through both"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a710552_29d0e11e","line":85,"in_reply_to":"5a710552_0ea2e7ae","updated":"2015-12-16 06:44:37.000000000","message":"I am still checking this one, test does not seem to pass without this.","commit_id":"b57a7a810d7fd635d7f2107d147ee0ab72e01216"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7a10ba36645654a9bc283d36870616a28d09361d","unresolved":false,"context_lines":[{"line_number":85,"context_line":"                                             source_container)"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"        # cycle container-sync"},{"line_number":88,"context_line":"        # We do this twice ot make sure we go through both"},{"line_number":89,"context_line":"        # sync phases. Otherwise, we may have a bad luck"},{"line_number":90,"context_line":"        # where the single node that needs to replicate"},{"line_number":91,"context_line":"        # hasn\u0027t got the update yet."}],"source_content_type":"text/x-python","patch_set":26,"id":"5a710552_ceca9fa1","line":88,"updated":"2015-12-15 17:38:10.000000000","message":"nit ot-\u003eto","commit_id":"b57a7a810d7fd635d7f2107d147ee0ab72e01216"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"399151b9656704a7e2ee6329e40bd896a2cc4b23","unresolved":false,"context_lines":[{"line_number":85,"context_line":"                                             source_container)"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"        # cycle container-sync"},{"line_number":88,"context_line":"        # We do this twice ot make sure we go through both"},{"line_number":89,"context_line":"        # sync phases. Otherwise, we may have a bad luck"},{"line_number":90,"context_line":"        # where the single node that needs to replicate"},{"line_number":91,"context_line":"        # hasn\u0027t got the update yet."}],"source_content_type":"text/x-python","patch_set":26,"id":"5a710552_69b589ec","line":88,"in_reply_to":"5a710552_ceca9fa1","updated":"2015-12-16 06:44:37.000000000","message":"yep","commit_id":"b57a7a810d7fd635d7f2107d147ee0ab72e01216"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7a10ba36645654a9bc283d36870616a28d09361d","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        # where the single node that needs to replicate"},{"line_number":91,"context_line":"        # hasn\u0027t got the update yet."},{"line_number":92,"context_line":"        Manager([\u0027container-sync\u0027]).once()"},{"line_number":93,"context_line":"        Manager([\u0027container-sync\u0027]).once()"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        # Allow Swift to commit container stale changes"},{"line_number":96,"context_line":"        headers, body \u003d client.get_container(self.url, self.token,"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a710552_2e932351","line":93,"updated":"2015-12-15 17:38:10.000000000","message":"see comment in the rst file - I don\u0027t understand this change.\n\nIt may be that the object updater should be run","commit_id":"b57a7a810d7fd635d7f2107d147ee0ab72e01216"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7a10ba36645654a9bc283d36870616a28d09361d","unresolved":false,"context_lines":[{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        # Allow Swift to commit container stale changes"},{"line_number":96,"context_line":"        headers, body \u003d client.get_container(self.url, self.token,"},{"line_number":97,"context_line":"                                             dest_container)"},{"line_number":98,"context_line":"        headers, body \u003d client.get_object(self.url, self.token,"},{"line_number":99,"context_line":"                                          dest_container, object_name)"},{"line_number":100,"context_line":"        self.assertEqual(body, \u0027test-body\u0027)"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a710552_6ed7ebed","line":97,"updated":"2015-12-15 17:38:10.000000000","message":"this change should not be necessary, the state of the container listing has no influence on the object get","commit_id":"b57a7a810d7fd635d7f2107d147ee0ab72e01216"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"399151b9656704a7e2ee6329e40bd896a2cc4b23","unresolved":false,"context_lines":[{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        # Allow Swift to commit container stale changes"},{"line_number":96,"context_line":"        headers, body \u003d client.get_container(self.url, self.token,"},{"line_number":97,"context_line":"                                             dest_container)"},{"line_number":98,"context_line":"        headers, body \u003d client.get_object(self.url, self.token,"},{"line_number":99,"context_line":"                                          dest_container, object_name)"},{"line_number":100,"context_line":"        self.assertEqual(body, \u0027test-body\u0027)"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a710552_09ee45ec","line":97,"in_reply_to":"5a710552_6ed7ebed","updated":"2015-12-16 06:44:37.000000000","message":"Agree. removed","commit_id":"b57a7a810d7fd635d7f2107d147ee0ab72e01216"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d3272fdaa3aa986b2e5ab3997a548cf06ecba54f","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        self.assertEqual(body, \u0027test-body\u0027)"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    def test_sync_lazy_skey(self):"},{"line_number":99,"context_line":"        # Create synced containers, but with no key at source"},{"line_number":100,"context_line":"        source_container, dest_container \u003d\\"},{"line_number":101,"context_line":"            self._setup_synced_containers(None, \u0027secret\u0027)"},{"line_number":102,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_51afe4e1","line":99,"updated":"2015-12-18 19:01:55.000000000","message":"nice refactoring and new tests!","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d3272fdaa3aa986b2e5ab3997a548cf06ecba54f","unresolved":false,"context_lines":[{"line_number":124,"context_line":"        self.assertEqual(body, \u0027test-body\u0027)"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"    def test_sync_lazy_dkey(self):"},{"line_number":127,"context_line":"        # Create synced containers, but with no key at source"},{"line_number":128,"context_line":"        source_container, dest_container \u003d\\"},{"line_number":129,"context_line":"            self._setup_synced_containers(\u0027secret\u0027, None)"},{"line_number":130,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_516844b9","line":127,"updated":"2015-12-18 19:01:55.000000000","message":"s/source/dest","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"133e8a97d28c0e1b328e9f7ade6cfdeeae57ae8e","unresolved":false,"context_lines":[{"line_number":124,"context_line":"        self.assertEqual(body, \u0027test-body\u0027)"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"    def test_sync_lazy_dkey(self):"},{"line_number":127,"context_line":"        # Create synced containers, but with no key at source"},{"line_number":128,"context_line":"        source_container, dest_container \u003d\\"},{"line_number":129,"context_line":"            self._setup_synced_containers(\u0027secret\u0027, None)"},{"line_number":130,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_c9f0046a","line":127,"in_reply_to":"3a7e1126_516844b9","updated":"2015-12-19 00:03:21.000000000","message":"done","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d3272fdaa3aa986b2e5ab3997a548cf06ecba54f","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        except ClientException as err:"},{"line_number":142,"context_line":"            self.assertEqual(err.http_status, HTTP_NOT_FOUND)"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        # amend source key"},{"line_number":145,"context_line":"        dest_headers \u003d {\u0027X-Container-Sync-Key\u0027: \u0027secret\u0027}"},{"line_number":146,"context_line":"        client.put_container(self.url, self.token, dest_container,"},{"line_number":147,"context_line":"                             headers\u003ddest_headers)"}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_115e3c12","line":144,"updated":"2015-12-18 19:01:55.000000000","message":"s/source/dest/","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"133e8a97d28c0e1b328e9f7ade6cfdeeae57ae8e","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        except ClientException as err:"},{"line_number":142,"context_line":"            self.assertEqual(err.http_status, HTTP_NOT_FOUND)"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        # amend source key"},{"line_number":145,"context_line":"        dest_headers \u003d {\u0027X-Container-Sync-Key\u0027: \u0027secret\u0027}"},{"line_number":146,"context_line":"        client.put_container(self.url, self.token, dest_container,"},{"line_number":147,"context_line":"                             headers\u003ddest_headers)"}],"source_content_type":"text/x-python","patch_set":27,"id":"3a7e1126_a9eb3877","line":144,"in_reply_to":"3a7e1126_115e3c12","updated":"2015-12-19 00:03:21.000000000","message":"done","commit_id":"725eb4ae192eb1df3478d54c1826d9f7b8321346"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4b85b8ceb32be975e46a15dbe0325c352af16e1e","unresolved":false,"context_lines":[{"line_number":108,"context_line":"        # cycle container-sync, nothing should happen"},{"line_number":109,"context_line":"        Manager([\u0027container-sync\u0027]).once()"},{"line_number":110,"context_line":"        try:"},{"line_number":111,"context_line":"            headers, body \u003d client.get_object(self.url, self.token,"},{"line_number":112,"context_line":"                                              dest_container, object_name)"},{"line_number":113,"context_line":"        except ClientException as err:"},{"line_number":114,"context_line":"            self.assertEqual(err.http_status, HTTP_NOT_FOUND)"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_f6efd484","line":111,"updated":"2016-01-05 16:58:35.000000000","message":"nit: headers body not used","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"1894a6f77a2cee4c903ef90cbea78e7e8b593469","unresolved":false,"context_lines":[{"line_number":108,"context_line":"        # cycle container-sync, nothing should happen"},{"line_number":109,"context_line":"        Manager([\u0027container-sync\u0027]).once()"},{"line_number":110,"context_line":"        try:"},{"line_number":111,"context_line":"            headers, body \u003d client.get_object(self.url, self.token,"},{"line_number":112,"context_line":"                                              dest_container, object_name)"},{"line_number":113,"context_line":"        except ClientException as err:"},{"line_number":114,"context_line":"            self.assertEqual(err.http_status, HTTP_NOT_FOUND)"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_228e7779","line":111,"in_reply_to":"fa69d971_f6efd484","updated":"2016-01-05 21:23:33.000000000","message":"Done","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4b85b8ceb32be975e46a15dbe0325c352af16e1e","unresolved":false,"context_lines":[{"line_number":111,"context_line":"            headers, body \u003d client.get_object(self.url, self.token,"},{"line_number":112,"context_line":"                                              dest_container, object_name)"},{"line_number":113,"context_line":"        except ClientException as err:"},{"line_number":114,"context_line":"            self.assertEqual(err.http_status, HTTP_NOT_FOUND)"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"        # amend source key"},{"line_number":117,"context_line":"        source_headers \u003d {\u0027X-Container-Sync-Key\u0027: \u0027secret\u0027}"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_76dd24df","line":114,"updated":"2016-01-05 16:58:35.000000000","message":"sorry I didn\u0027t spot this before - test passes if exception is not raised.\n\nUse same pattern as elsewhere:\n\n  self.assertRaises(ClientException) as cm:\n      client.get_object(self.url, self.token, dest_container, object_name)\n  self.assertEqual(cm.exception.http_status, HTTP_NOT_FOUND)","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"1894a6f77a2cee4c903ef90cbea78e7e8b593469","unresolved":false,"context_lines":[{"line_number":111,"context_line":"            headers, body \u003d client.get_object(self.url, self.token,"},{"line_number":112,"context_line":"                                              dest_container, object_name)"},{"line_number":113,"context_line":"        except ClientException as err:"},{"line_number":114,"context_line":"            self.assertEqual(err.http_status, HTTP_NOT_FOUND)"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"        # amend source key"},{"line_number":117,"context_line":"        source_headers \u003d {\u0027X-Container-Sync-Key\u0027: \u0027secret\u0027}"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_428b6b68","line":114,"in_reply_to":"fa69d971_76dd24df","updated":"2016-01-05 21:23:33.000000000","message":"good catch. Done","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4b85b8ceb32be975e46a15dbe0325c352af16e1e","unresolved":false,"context_lines":[{"line_number":138,"context_line":"        try:"},{"line_number":139,"context_line":"            headers, body \u003d client.get_object(self.url, self.token,"},{"line_number":140,"context_line":"                                              dest_container, object_name)"},{"line_number":141,"context_line":"        except ClientException as err:"},{"line_number":142,"context_line":"            self.assertEqual(err.http_status, HTTP_NOT_FOUND)"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        # amend dest key"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_36fa4c44","line":141,"updated":"2016-01-05 16:58:35.000000000","message":"same as above","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"1894a6f77a2cee4c903ef90cbea78e7e8b593469","unresolved":false,"context_lines":[{"line_number":138,"context_line":"        try:"},{"line_number":139,"context_line":"            headers, body \u003d client.get_object(self.url, self.token,"},{"line_number":140,"context_line":"                                              dest_container, object_name)"},{"line_number":141,"context_line":"        except ClientException as err:"},{"line_number":142,"context_line":"            self.assertEqual(err.http_status, HTTP_NOT_FOUND)"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        # amend dest key"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_e2ea9f0e","line":141,"in_reply_to":"fa69d971_36fa4c44","updated":"2016-01-05 21:23:33.000000000","message":"Done","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"}],"test/unit/container/test_replicator.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a20455f7416ea5176823547eff8b278eeaa5e1d1","unresolved":false,"context_lines":[{"line_number":1076,"context_line":"            self.assertEquals(remove_sync[0], 0)"},{"line_number":1077,"context_line":"            part, node \u003d self._get_broker_part_node(broker_2)"},{"line_number":1078,"context_line":"            self._run_once(node)"},{"line_number":1079,"context_line":"            self.assertEquals(add_sync[0], 2)"},{"line_number":1080,"context_line":"            self.assertEquals(remove_sync[0], 0)"},{"line_number":1081,"context_line":""},{"line_number":1082,"context_line":"            broker_2.update_metadata("}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_6ffd1eb0","line":1079,"updated":"2015-08-18 19:15:04.000000000","message":"nit: i think these asserts would be stronger if it was a list instead of a count","commit_id":"aefd20882642662b4f7deeb19ddfa9617cc8c2ff"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":1017,"context_line":"            {\u0027X-Container-Sync-To\u0027: (\u0027a\u0027, timestamp.internal)})"},{"line_number":1018,"context_line":"        try:"},{"line_number":1019,"context_line":"            daemon._post_replicate_hook(broker, info, [])"},{"line_number":1020,"context_line":"        except OSError as oserr:"},{"line_number":1021,"context_line":"            self.assertEqual(oserr.errno, 1)"},{"line_number":1022,"context_line":""},{"line_number":1023,"context_line":"    def test_update_sync_store(self):"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_ef3d8bc4","line":1020,"updated":"2015-12-21 17:01:17.000000000","message":"the exception is not raised, test always passes.","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":1017,"context_line":"            {\u0027X-Container-Sync-To\u0027: (\u0027a\u0027, timestamp.internal)})"},{"line_number":1018,"context_line":"        try:"},{"line_number":1019,"context_line":"            daemon._post_replicate_hook(broker, info, [])"},{"line_number":1020,"context_line":"        except OSError as oserr:"},{"line_number":1021,"context_line":"            self.assertEqual(oserr.errno, 1)"},{"line_number":1022,"context_line":""},{"line_number":1023,"context_line":"    def test_update_sync_store(self):"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_594e6c97","line":1020,"in_reply_to":"3a7e1126_ef3d8bc4","updated":"2016-01-01 21:44:24.000000000","message":"right. Adopted your diff.","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":1029,"context_line":"                if \u0027X-Container-Sync-To\u0027 not in broker.metadata:"},{"line_number":1030,"context_line":"                    return"},{"line_number":1031,"context_line":"                sync_to \u003d broker.metadata[\u0027X-Container-Sync-To\u0027][0]"},{"line_number":1032,"context_line":"                if broker.is_deleted() or sync_to \u003d\u003d \u0027\u0027:"},{"line_number":1033,"context_line":"                    remove_sync.append(broker.db_file)"},{"line_number":1034,"context_line":"                else:"},{"line_number":1035,"context_line":"                    add_sync.append(broker.db_file)"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_af4a2353","line":1032,"updated":"2015-12-21 17:01:17.000000000","message":"this logic no longer matches that of the sync_store w.r.t. sync_key being set or not.\n\nBetter would be to leave the sync_store.update_sync_store method intact and assert whether it calls the add_ or remove_ methods.","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":1029,"context_line":"                if \u0027X-Container-Sync-To\u0027 not in broker.metadata:"},{"line_number":1030,"context_line":"                    return"},{"line_number":1031,"context_line":"                sync_to \u003d broker.metadata[\u0027X-Container-Sync-To\u0027][0]"},{"line_number":1032,"context_line":"                if broker.is_deleted() or sync_to \u003d\u003d \u0027\u0027:"},{"line_number":1033,"context_line":"                    remove_sync.append(broker.db_file)"},{"line_number":1034,"context_line":"                else:"},{"line_number":1035,"context_line":"                    add_sync.append(broker.db_file)"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_5da18d93","line":1032,"in_reply_to":"3a7e1126_af4a2353","updated":"2016-01-01 21:44:24.000000000","message":"Right. Fixed along those lines.","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":1062,"context_line":"        add_sync \u003d list()"},{"line_number":1063,"context_line":"        remove_sync \u003d list()"},{"line_number":1064,"context_line":"        daemon._post_replicate_hook(broker, info, [])"},{"line_number":1065,"context_line":"        self.assertTrue(broker.db_file in add_sync)"},{"line_number":1066,"context_line":"        self.assertTrue(broker.db_file not in remove_sync)"},{"line_number":1067,"context_line":""},{"line_number":1068,"context_line":"        timestamp \u003d next(ts_iter)"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_6f775ba0","line":1065,"updated":"2015-12-21 17:01:17.000000000","message":"this should not happen though because sync_key is not set??","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":1062,"context_line":"        add_sync \u003d list()"},{"line_number":1063,"context_line":"        remove_sync \u003d list()"},{"line_number":1064,"context_line":"        daemon._post_replicate_hook(broker, info, [])"},{"line_number":1065,"context_line":"        self.assertTrue(broker.db_file in add_sync)"},{"line_number":1066,"context_line":"        self.assertTrue(broker.db_file not in remove_sync)"},{"line_number":1067,"context_line":""},{"line_number":1068,"context_line":"        timestamp \u003d next(ts_iter)"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_bd04497e","line":1065,"in_reply_to":"3a7e1126_6f775ba0","updated":"2016-01-01 21:44:24.000000000","message":"right. changed accordingly.","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":1089,"context_line":"                if \u0027X-Container-Sync-To\u0027 not in broker.metadata:"},{"line_number":1090,"context_line":"                    return"},{"line_number":1091,"context_line":"                sync_to \u003d broker.metadata[\u0027X-Container-Sync-To\u0027][0]"},{"line_number":1092,"context_line":"                if broker.is_deleted() or sync_to \u003d\u003d \u0027\u0027:"},{"line_number":1093,"context_line":"                    self.remove_synced_container(broker, False)"},{"line_number":1094,"context_line":"                else:"},{"line_number":1095,"context_line":"                    self.add_synced_container(broker)"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_8f7ac786","line":1092,"updated":"2015-12-21 17:01:17.000000000","message":"as above, this is not really mocking the actual sync_store behavior","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":1089,"context_line":"                if \u0027X-Container-Sync-To\u0027 not in broker.metadata:"},{"line_number":1090,"context_line":"                    return"},{"line_number":1091,"context_line":"                sync_to \u003d broker.metadata[\u0027X-Container-Sync-To\u0027][0]"},{"line_number":1092,"context_line":"                if broker.is_deleted() or sync_to \u003d\u003d \u0027\u0027:"},{"line_number":1093,"context_line":"                    self.remove_synced_container(broker, False)"},{"line_number":1094,"context_line":"                else:"},{"line_number":1095,"context_line":"                    self.add_synced_container(broker)"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_3d27f913","line":1092,"in_reply_to":"3a7e1126_8f7ac786","updated":"2016-01-01 21:44:24.000000000","message":"yep. same type of fix","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":1138,"context_line":"            add_sync_assertions()"},{"line_number":1139,"context_line":"            remove_sync_assertions(False)"},{"line_number":1140,"context_line":"            # \"replicate\""},{"line_number":1141,"context_line":"            part, node \u003d self._get_broker_part_node(broker_2)"},{"line_number":1142,"context_line":"            self._run_once(node)"},{"line_number":1143,"context_line":"            add_sync_assertions()"},{"line_number":1144,"context_line":"            remove_sync_assertions(False)"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_2f6dd3cb","line":1141,"updated":"2015-12-21 17:01:17.000000000","message":"The way the FakeRing is setup, both brokers will have same partition (0) and node, so it is sufficient to call run_once just once","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":1138,"context_line":"            add_sync_assertions()"},{"line_number":1139,"context_line":"            remove_sync_assertions(False)"},{"line_number":1140,"context_line":"            # \"replicate\""},{"line_number":1141,"context_line":"            part, node \u003d self._get_broker_part_node(broker_2)"},{"line_number":1142,"context_line":"            self._run_once(node)"},{"line_number":1143,"context_line":"            add_sync_assertions()"},{"line_number":1144,"context_line":"            remove_sync_assertions(False)"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_c8d5b943","line":1141,"in_reply_to":"3a7e1126_2f6dd3cb","updated":"2016-01-01 21:44:24.000000000","message":"Done. I did hesitate here, as I thought that FakeRing change is likely to happen. On the other hand I assume that there will be dozens other places that would need to get adjusted.\nSo changed and added some remarks","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"}],"test/unit/container/test_server.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":1165,"context_line":"        self._create_synced_container_and_validate_sync_store()"},{"line_number":1166,"context_line":"        self._remove_sync_and_validate_sync_store(\u0027DELETE\u0027)"},{"line_number":1167,"context_line":""},{"line_number":1168,"context_line":"    def _create_synced_container_and_validate_sync_store(self):"},{"line_number":1169,"context_line":"        req \u003d Request.blank("},{"line_number":1170,"context_line":"            \u0027/sda1/p/a/c\u0027, environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027PUT\u0027},"},{"line_number":1171,"context_line":"            headers\u003d{\u0027x-timestamp\u0027: \u00271\u0027,"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_1534a447","line":1168,"updated":"2015-12-21 17:01:17.000000000","message":"add a test that sync_store symlink gets created if metadata is updated with a POST - perhaps by parameterising this helper method to take a method arg\u003d[PUT|POST]","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":1165,"context_line":"        self._create_synced_container_and_validate_sync_store()"},{"line_number":1166,"context_line":"        self._remove_sync_and_validate_sync_store(\u0027DELETE\u0027)"},{"line_number":1167,"context_line":""},{"line_number":1168,"context_line":"    def _create_synced_container_and_validate_sync_store(self):"},{"line_number":1169,"context_line":"        req \u003d Request.blank("},{"line_number":1170,"context_line":"            \u0027/sda1/p/a/c\u0027, environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027PUT\u0027},"},{"line_number":1171,"context_line":"            headers\u003d{\u0027x-timestamp\u0027: \u00271\u0027,"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_e8e51da8","line":1168,"in_reply_to":"3a7e1126_1534a447","updated":"2016-01-01 21:44:24.000000000","message":"Done. While at it got for free a test validating no link is created when a non synced container is created","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4b85b8ceb32be975e46a15dbe0325c352af16e1e","unresolved":false,"context_lines":[{"line_number":1194,"context_line":"        db \u003d self.controller._get_container_broker(\u0027sda1\u0027, \u0027p\u0027, \u0027a\u0027, \u0027c\u0027)"},{"line_number":1195,"context_line":"        sync_store \u003d self.controller.sync_store"},{"line_number":1196,"context_line":"        db_path \u003d db.db_file"},{"line_number":1197,"context_line":"        db_link \u003d sync_store._container_to_synced_container_path(db_path)"},{"line_number":1198,"context_line":"        self.assertTrue(os.path.exists(db_link))"},{"line_number":1199,"context_line":""},{"line_number":1200,"context_line":"    def _remove_sync_and_validate_sync_store(self, method):"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_79d0b71f","line":1197,"updated":"2016-01-05 16:58:35.000000000","message":"nit: no need to declare db_path variable, just use db.db_file as arg","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"1894a6f77a2cee4c903ef90cbea78e7e8b593469","unresolved":false,"context_lines":[{"line_number":1194,"context_line":"        db \u003d self.controller._get_container_broker(\u0027sda1\u0027, \u0027p\u0027, \u0027a\u0027, \u0027c\u0027)"},{"line_number":1195,"context_line":"        sync_store \u003d self.controller.sync_store"},{"line_number":1196,"context_line":"        db_path \u003d db.db_file"},{"line_number":1197,"context_line":"        db_link \u003d sync_store._container_to_synced_container_path(db_path)"},{"line_number":1198,"context_line":"        self.assertTrue(os.path.exists(db_link))"},{"line_number":1199,"context_line":""},{"line_number":1200,"context_line":"    def _remove_sync_and_validate_sync_store(self, method):"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_8253a36a","line":1197,"in_reply_to":"fa69d971_79d0b71f","updated":"2016-01-05 21:23:33.000000000","message":"well, I did go for the below suggestion.","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4b85b8ceb32be975e46a15dbe0325c352af16e1e","unresolved":false,"context_lines":[{"line_number":1195,"context_line":"        sync_store \u003d self.controller.sync_store"},{"line_number":1196,"context_line":"        db_path \u003d db.db_file"},{"line_number":1197,"context_line":"        db_link \u003d sync_store._container_to_synced_container_path(db_path)"},{"line_number":1198,"context_line":"        self.assertTrue(os.path.exists(db_link))"},{"line_number":1199,"context_line":""},{"line_number":1200,"context_line":"    def _remove_sync_and_validate_sync_store(self, method):"},{"line_number":1201,"context_line":"        if method \u003d\u003d \u0027DELETE\u0027:"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_792757ca","line":1198,"updated":"2016-01-05 16:58:35.000000000","message":"A nice addition would be:\n\n  locations \u003d [l for l in sync_store.synced_containers_generator()]\n      self.assertEqual(1, len(locations))\n      self.assertEqual(db_path, locations[0])\n\nin which case db_path is needed after all :)","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"1894a6f77a2cee4c903ef90cbea78e7e8b593469","unresolved":false,"context_lines":[{"line_number":1195,"context_line":"        sync_store \u003d self.controller.sync_store"},{"line_number":1196,"context_line":"        db_path \u003d db.db_file"},{"line_number":1197,"context_line":"        db_link \u003d sync_store._container_to_synced_container_path(db_path)"},{"line_number":1198,"context_line":"        self.assertTrue(os.path.exists(db_link))"},{"line_number":1199,"context_line":""},{"line_number":1200,"context_line":"    def _remove_sync_and_validate_sync_store(self, method):"},{"line_number":1201,"context_line":"        if method \u003d\u003d \u0027DELETE\u0027:"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_c2df5b38","line":1198,"in_reply_to":"fa69d971_792757ca","updated":"2016-01-05 21:23:33.000000000","message":"Nice addition. Added","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4b85b8ceb32be975e46a15dbe0325c352af16e1e","unresolved":false,"context_lines":[{"line_number":1213,"context_line":"        sync_store \u003d self.controller.sync_store"},{"line_number":1214,"context_line":"        db_path \u003d db.db_file"},{"line_number":1215,"context_line":"        db_link \u003d sync_store._container_to_synced_container_path(db_path)"},{"line_number":1216,"context_line":"        self.assertFalse(os.path.exists(db_link))"},{"line_number":1217,"context_line":""},{"line_number":1218,"context_line":"    def test_REPLICATE_insufficient_storage(self):"},{"line_number":1219,"context_line":"        conf \u003d {\u0027devices\u0027: self.testdir, \u0027mount_check\u0027: \u0027true\u0027}"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_dc43d111","line":1216,"updated":"2016-01-05 16:58:35.000000000","message":"maybe \n\n    sync_containers \u003d [c for c in sync_store.synced_containers_generator()]\n    self.assertFalse(sync_containers)","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"1894a6f77a2cee4c903ef90cbea78e7e8b593469","unresolved":false,"context_lines":[{"line_number":1213,"context_line":"        sync_store \u003d self.controller.sync_store"},{"line_number":1214,"context_line":"        db_path \u003d db.db_file"},{"line_number":1215,"context_line":"        db_link \u003d sync_store._container_to_synced_container_path(db_path)"},{"line_number":1216,"context_line":"        self.assertFalse(os.path.exists(db_link))"},{"line_number":1217,"context_line":""},{"line_number":1218,"context_line":"    def test_REPLICATE_insufficient_storage(self):"},{"line_number":1219,"context_line":"        conf \u003d {\u0027devices\u0027: self.testdir, \u0027mount_check\u0027: \u0027true\u0027}"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_626f8f2b","line":1216,"in_reply_to":"fa69d971_dc43d111","updated":"2016-01-05 21:23:33.000000000","message":"Done","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"}],"test/unit/container/test_sync.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":237,"context_line":"                raise Exception(\u0027we are now done\u0027)"},{"line_number":238,"context_line":"            return returns[time_calls[0] - 1]"},{"line_number":239,"context_line":""},{"line_number":240,"context_line":"        class FakeSyncedContainersStore(object):"},{"line_number":241,"context_line":"            def synced_containers_generator(self):"},{"line_number":242,"context_line":"                synced_containers_calls[0] +\u003d 1"},{"line_number":243,"context_line":"                # Makes .container_sync() short-circuit"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_4fa7ff0a","line":240,"updated":"2015-12-21 17:01:17.000000000","message":"this appears to duplicate the Class at line 182 - could be moved to module level and shared?","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":237,"context_line":"                raise Exception(\u0027we are now done\u0027)"},{"line_number":238,"context_line":"            return returns[time_calls[0] - 1]"},{"line_number":239,"context_line":""},{"line_number":240,"context_line":"        class FakeSyncedContainersStore(object):"},{"line_number":241,"context_line":"            def synced_containers_generator(self):"},{"line_number":242,"context_line":"                synced_containers_calls[0] +\u003d 1"},{"line_number":243,"context_line":"                # Makes .container_sync() short-circuit"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_a30506c3","line":240,"in_reply_to":"3a7e1126_4fa7ff0a","updated":"2016-01-01 21:44:24.000000000","message":"Actually I got rid of that following the eye opening examples of yours :-)","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":271,"context_line":"        self.assertEqual(synced_containers_calls, [2])"},{"line_number":272,"context_line":"        self.assertEqual(cs.reported, 3604)"},{"line_number":273,"context_line":""},{"line_number":274,"context_line":"    def test_container_sync_not_db(self):"},{"line_number":275,"context_line":"        cring \u003d FakeRing()"},{"line_number":276,"context_line":"        with mock.patch(\u0027swift.container.sync.InternalClient\u0027):"},{"line_number":277,"context_line":"            cs \u003d sync.ContainerSync({}, container_ring\u003dcring)"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_cf48cfb5","line":274,"updated":"2015-12-21 17:01:17.000000000","message":"hmmm, what does this test verify?!?","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":271,"context_line":"        self.assertEqual(synced_containers_calls, [2])"},{"line_number":272,"context_line":"        self.assertEqual(cs.reported, 3604)"},{"line_number":273,"context_line":""},{"line_number":274,"context_line":"    def test_container_sync_not_db(self):"},{"line_number":275,"context_line":"        cring \u003d FakeRing()"},{"line_number":276,"context_line":"        with mock.patch(\u0027swift.container.sync.InternalClient\u0027):"},{"line_number":277,"context_line":"            cs \u003d sync.ContainerSync({}, container_ring\u003dcring)"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_c3e27ad5","line":274,"in_reply_to":"3a7e1126_cf48cfb5","updated":"2016-01-01 21:44:24.000000000","message":"I am not sure really. The __init__ function can raise SysExit or IOError. I guess this validates that this is not the case. It still does not explain the name of the function.\nLooks like an old heritage.","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":277,"context_line":"            cs \u003d sync.ContainerSync({}, container_ring\u003dcring)"},{"line_number":278,"context_line":"        self.assertEqual(cs.container_failures, 0)"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"    def test_container_sync_missing_db(self):"},{"line_number":281,"context_line":"        cring \u003d FakeRing()"},{"line_number":282,"context_line":"        with mock.patch(\u0027swift.container.sync.InternalClient\u0027):"},{"line_number":283,"context_line":"            cs \u003d sync.ContainerSync({}, container_ring\u003dcring)"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_d5a61cad","line":280,"updated":"2015-12-21 17:01:17.000000000","message":"I think this test can be simplified using mock.patch - see my diff","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":277,"context_line":"            cs \u003d sync.ContainerSync({}, container_ring\u003dcring)"},{"line_number":278,"context_line":"        self.assertEqual(cs.container_failures, 0)"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"    def test_container_sync_missing_db(self):"},{"line_number":281,"context_line":"        cring \u003d FakeRing()"},{"line_number":282,"context_line":"        with mock.patch(\u0027swift.container.sync.InternalClient\u0027):"},{"line_number":283,"context_line":"            cs \u003d sync.ContainerSync({}, container_ring\u003dcring)"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_f87161a9","line":280,"in_reply_to":"3a7e1126_d5a61cad","updated":"2016-01-01 21:44:24.000000000","message":"Absolutly.","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":290,"context_line":"            def get_info(self):"},{"line_number":291,"context_line":"                raise DatabaseConnectionError(\u0027a\u0027, \"DB doesn\u0027t exist\")"},{"line_number":292,"context_line":""},{"line_number":293,"context_line":"        class FakeSyncedContainersStore(object):"},{"line_number":294,"context_line":"            def remove_synced_container(self, broker):"},{"line_number":295,"context_line":"                remove_synced_container_called[0] +\u003d 1"},{"line_number":296,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_9a4a0354","line":293,"updated":"2015-12-21 17:01:17.000000000","message":"see examples for mock.patch of remove_synced_container in my diff for test_replicator - can use similar here","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":290,"context_line":"            def get_info(self):"},{"line_number":291,"context_line":"                raise DatabaseConnectionError(\u0027a\u0027, \"DB doesn\u0027t exist\")"},{"line_number":292,"context_line":""},{"line_number":293,"context_line":"        class FakeSyncedContainersStore(object):"},{"line_number":294,"context_line":"            def remove_synced_container(self, broker):"},{"line_number":295,"context_line":"                remove_synced_container_called[0] +\u003d 1"},{"line_number":296,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_9954e421","line":293,"in_reply_to":"3a7e1126_9a4a0354","updated":"2016-01-01 21:44:24.000000000","message":"right. Adopted your diff","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":301,"context_line":"            \u0027x_container_sync_point2\u0027: -1})"},{"line_number":302,"context_line":""},{"line_number":303,"context_line":"        orig_ContainerBroker \u003d sync.ContainerBroker"},{"line_number":304,"context_line":"        try:"},{"line_number":305,"context_line":"            sync.ContainerBroker \u003d lambda p: fcb"},{"line_number":306,"context_line":"            cs.sync_store \u003d FakeSyncedContainersStore()"},{"line_number":307,"context_line":"            cs.container_sync(\u0027isa.db\u0027)"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_af9223f4","line":304,"updated":"2015-12-21 17:01:17.000000000","message":"use mock.patch rather than try/patch/finally\n\n  with mock.patch(\u0027.....ContainerBroker.get_info\u0027) as fake_get_info:\n      fake_get_info.side_effect \u003d DatabaseConnectionError(\u0027a\u0027, \"DB doesn\u0027t exist\")\n      etc...","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":301,"context_line":"            \u0027x_container_sync_point2\u0027: -1})"},{"line_number":302,"context_line":""},{"line_number":303,"context_line":"        orig_ContainerBroker \u003d sync.ContainerBroker"},{"line_number":304,"context_line":"        try:"},{"line_number":305,"context_line":"            sync.ContainerBroker \u003d lambda p: fcb"},{"line_number":306,"context_line":"            cs.sync_store \u003d FakeSyncedContainersStore()"},{"line_number":307,"context_line":"            cs.container_sync(\u0027isa.db\u0027)"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_986945c1","line":304,"in_reply_to":"3a7e1126_af9223f4","updated":"2016-01-01 21:44:24.000000000","message":"These mock patch tricks are eye openers for me...","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":347,"context_line":""},{"line_number":348,"context_line":"        # Test the case where get_info returns DatabaseConnectionError"},{"line_number":349,"context_line":"        # error different than DB does not exist"},{"line_number":350,"context_line":"        class FakeContainerBrokerInfo(FakeContainerBroker):"},{"line_number":351,"context_line":"            def get_info(self):"},{"line_number":352,"context_line":"                raise DatabaseConnectionError(\u0027a\u0027, \u0027a\u0027)"},{"line_number":353,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_956174d5","line":350,"updated":"2015-12-21 17:01:17.000000000","message":"re-using same class name as above - might be confusing.\n\nI wonder if the classes can be avoided by mocking get_info?","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":347,"context_line":""},{"line_number":348,"context_line":"        # Test the case where get_info returns DatabaseConnectionError"},{"line_number":349,"context_line":"        # error different than DB does not exist"},{"line_number":350,"context_line":"        class FakeContainerBrokerInfo(FakeContainerBroker):"},{"line_number":351,"context_line":"            def get_info(self):"},{"line_number":352,"context_line":"                raise DatabaseConnectionError(\u0027a\u0027, \u0027a\u0027)"},{"line_number":353,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_b86449e5","line":350,"in_reply_to":"3a7e1126_956174d5","updated":"2016-01-01 21:44:24.000000000","message":"right, done.","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4b85b8ceb32be975e46a15dbe0325c352af16e1e","unresolved":false,"context_lines":[{"line_number":84,"context_line":""},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"@count"},{"line_number":87,"context_line":"def fake_generator(self):"},{"line_number":88,"context_line":"    # Makes .container_sync() short-circuit"},{"line_number":89,"context_line":"    yield \u0027container.db\u0027"},{"line_number":90,"context_line":"    return"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_c56bf0b7","line":87,"updated":"2016-01-05 16:58:35.000000000","message":"we can use mock.patch side_effect to implement a fake generator - see diff","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"1894a6f77a2cee4c903ef90cbea78e7e8b593469","unresolved":false,"context_lines":[{"line_number":84,"context_line":""},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"@count"},{"line_number":87,"context_line":"def fake_generator(self):"},{"line_number":88,"context_line":"    # Makes .container_sync() short-circuit"},{"line_number":89,"context_line":"    yield \u0027container.db\u0027"},{"line_number":90,"context_line":"    return"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_c7152a40","line":87,"in_reply_to":"fa69d971_c56bf0b7","updated":"2016-01-05 21:23:33.000000000","message":"yep. got it. This mock module is something, and I guess this is just the tip of the iceberg...","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4b85b8ceb32be975e46a15dbe0325c352af16e1e","unresolved":false,"context_lines":[{"line_number":197,"context_line":""},{"line_number":198,"context_line":"        gen_func \u003d (\u0027swift.container.sync_store.\u0027"},{"line_number":199,"context_line":"                    \u0027ContainerSyncStore.synced_containers_generator\u0027)"},{"line_number":200,"context_line":"        with nested("},{"line_number":201,"context_line":"            mock.patch(\u0027swift.container.sync.InternalClient\u0027),"},{"line_number":202,"context_line":"            mock.patch(\u0027swift.container.sync.time\u0027, fake_time),"},{"line_number":203,"context_line":"            mock.patch(\u0027swift.container.sync.sleep\u0027, fake_sleep),"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_fc19f588","line":200,"updated":"2016-01-05 16:58:35.000000000","message":"contextlib.nested does not exist in python 3 :( see https://review.openstack.org/#/c/238237","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"1894a6f77a2cee4c903ef90cbea78e7e8b593469","unresolved":false,"context_lines":[{"line_number":197,"context_line":""},{"line_number":198,"context_line":"        gen_func \u003d (\u0027swift.container.sync_store.\u0027"},{"line_number":199,"context_line":"                    \u0027ContainerSyncStore.synced_containers_generator\u0027)"},{"line_number":200,"context_line":"        with nested("},{"line_number":201,"context_line":"            mock.patch(\u0027swift.container.sync.InternalClient\u0027),"},{"line_number":202,"context_line":"            mock.patch(\u0027swift.container.sync.time\u0027, fake_time),"},{"line_number":203,"context_line":"            mock.patch(\u0027swift.container.sync.sleep\u0027, fake_sleep),"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_82af2329","line":200,"in_reply_to":"fa69d971_fc19f588","updated":"2016-01-05 21:23:33.000000000","message":"and I was disappointed that nested does not work with mock.patch ... as...","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4b85b8ceb32be975e46a15dbe0325c352af16e1e","unresolved":false,"context_lines":[{"line_number":244,"context_line":""},{"line_number":245,"context_line":"        gen_func \u003d (\u0027swift.container.sync_store.\u0027"},{"line_number":246,"context_line":"                    \u0027ContainerSyncStore.synced_containers_generator\u0027)"},{"line_number":247,"context_line":"        with nested("},{"line_number":248,"context_line":"            mock.patch(\u0027swift.container.sync.InternalClient\u0027),"},{"line_number":249,"context_line":"            mock.patch(\u0027swift.container.sync.time\u0027, fake_time),"},{"line_number":250,"context_line":"            mock.patch(gen_func, fake_generator),"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_0506e885","line":247,"updated":"2016-01-05 16:58:35.000000000","message":"as above, nested not in py 3","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"1894a6f77a2cee4c903ef90cbea78e7e8b593469","unresolved":false,"context_lines":[{"line_number":244,"context_line":""},{"line_number":245,"context_line":"        gen_func \u003d (\u0027swift.container.sync_store.\u0027"},{"line_number":246,"context_line":"                    \u0027ContainerSyncStore.synced_containers_generator\u0027)"},{"line_number":247,"context_line":"        with nested("},{"line_number":248,"context_line":"            mock.patch(\u0027swift.container.sync.InternalClient\u0027),"},{"line_number":249,"context_line":"            mock.patch(\u0027swift.container.sync.time\u0027, fake_time),"},{"line_number":250,"context_line":"            mock.patch(gen_func, fake_generator),"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_c40e883a","line":247,"in_reply_to":"fa69d971_0506e885","updated":"2016-01-05 21:23:33.000000000","message":"Done","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"6a5742867bca1c90231e38d703e9eb5e21876b34","unresolved":false,"context_lines":[{"line_number":202,"context_line":"            self.assertLessEqual(sleep_calls[0], cs.interval)"},{"line_number":203,"context_line":"            self.assertEqual(cs.interval - 1, sleep_calls[1])"},{"line_number":204,"context_line":"            self.assertEqual(2, fake_generator.call_count)"},{"line_number":205,"context_line":"            self.assertEqual(cs.reported, 3602)"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    def test_run_once(self):"},{"line_number":208,"context_line":"        # This runs runs_once with fakes twice, the first causing an interim"}],"source_content_type":"text/x-python","patch_set":30,"id":"fa69d971_e1fb7379","line":205,"updated":"2016-01-06 00:28:28.000000000","message":"when using muliline before the start of a code block (:), you should probably add more then 1 indent of space, otherwise the code is hard to read. I.e the lines of the \u0027:\u0027 statement line up with the code underneath. To make it more readable it should be more like:\n\n  with mock.patch(\u0027swift.container.sync.InternalClient\u0027), \\                                                                                                                                                                                                     \n          mock.patch(\u0027swift.container.sync.time\u0027, fake_time), \\                                                                                                                                                                                                 \n          mock.patch(\u0027swift.container.sync.sleep\u0027, fake_sleep), \\                                                                                                                                                                                               \n          mock.patch(gen_func) as fake_generator, \\                                                                                                                                                                                                             \n          mock.patch(\u0027swift.container.sync.ContainerBroker\u0027,                                                                                                                                                                                                    \n                     lambda p: FakeContainerBroker(p, info\u003d{                                                                                                                                                                                                       \n                         \u0027account\u0027: \u0027a\u0027, \u0027container\u0027: \u0027c\u0027,                                                                                                                                                                                                         \n                         \u0027storage_policy_index\u0027: 0})):                                                                                                                                                                                                                 \n      fake_generator.side_effect \u003d [iter([\u0027container.db\u0027]),                                                                                                                                                                                                         \n                                      iter([\u0027container.db\u0027])]                                                                                                                                                                                                   \n      cs \u003d sync.ContainerSync({}, container_ring\u003dFakeRing())                                                                                                                                                                                                    \n      try:                                                                                                                                                                                                                                                      \n          cs.run_forever()                                                                                                                                                                                                                                      \n      except Exception as err:                                                                                                                                                                                                                                  \n          if str(err) !\u003d \u0027we are now done\u0027:                                                                                                                                                                                                                     \n              raise                                                                                                                                                                                                                                             \n                                                                                                                                                                                                                                                              \n      self.assertEqual(time_calls, [9])                                                                                                                                                                                                                         \n      self.assertEqual(len(sleep_calls), 2)                                                                                                                                                                                                                     \n      self.assertLessEqual(sleep_calls[0], cs.interval)                                                                                                                                                                                                         \n      self.assertEqual(cs.interval - 1, sleep_calls[1])                                                                                                                                                                                                         \n      self.assertEqual(2, fake_generator.call_count)                                                                                                                                                                                                            \n      self.assertEqual(cs.reported, 3602)","commit_id":"2072293e356e4f7122b07e7093f260a6f2b710de"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"23805afb3dcb87a047c31272684ba032ef89debc","unresolved":false,"context_lines":[{"line_number":233,"context_line":"            mock.patch(\u0027swift.container.sync.ContainerBroker\u0027,"},{"line_number":234,"context_line":"                       lambda p: FakeContainerBroker(p, info\u003d{"},{"line_number":235,"context_line":"                           \u0027account\u0027: \u0027a\u0027, \u0027container\u0027: \u0027c\u0027,"},{"line_number":236,"context_line":"                           \u0027storage_policy_index\u0027: 0})):"},{"line_number":237,"context_line":"            fake_generator.side_effect \u003d [iter([\u0027container.db\u0027]),"},{"line_number":238,"context_line":"                                          iter([\u0027container.db\u0027])]"},{"line_number":239,"context_line":"            cs \u003d sync.ContainerSync({}, container_ring\u003dFakeRing())"}],"source_content_type":"text/x-python","patch_set":30,"id":"fa69d971_10c35e8a","line":236,"updated":"2016-01-06 16:06:09.000000000","message":"Matt is going to ding you for indents here too :) as above","commit_id":"2072293e356e4f7122b07e7093f260a6f2b710de"}],"test/unit/container/test_sync_store.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a20455f7416ea5176823547eff8b278eeaa5e1d1","unresolved":false,"context_lines":[{"line_number":107,"context_line":"        sds \u003d sync_store.ContainerSyncStore(self.devices_dir, self.logger)"},{"line_number":108,"context_line":"        cfile \u003d self.pick_dbfile()"},{"line_number":109,"context_line":"        broker \u003d FakeContainerBroker(cfile)"},{"line_number":110,"context_line":"        for i in range(2):"},{"line_number":111,"context_line":"            res \u003d sds.add_synced_container(broker)"},{"line_number":112,"context_line":"            self.assertTrue(res)"},{"line_number":113,"context_line":"            scpath \u003d sds._container_to_synced_container_path(cfile)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_b4005b35","line":110,"updated":"2015-08-18 19:15:04.000000000","message":"the loop two asserts that it\u0027s atomic yes?","commit_id":"aefd20882642662b4f7deeb19ddfa9617cc8c2ff"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a20455f7416ea5176823547eff8b278eeaa5e1d1","unresolved":false,"context_lines":[{"line_number":109,"context_line":"        broker \u003d FakeContainerBroker(cfile)"},{"line_number":110,"context_line":"        for i in range(2):"},{"line_number":111,"context_line":"            res \u003d sds.add_synced_container(broker)"},{"line_number":112,"context_line":"            self.assertTrue(res)"},{"line_number":113,"context_line":"            scpath \u003d sds._container_to_synced_container_path(cfile)"},{"line_number":114,"context_line":"            with open(scpath, \u0027r\u0027) as infile:"},{"line_number":115,"context_line":"                self.assertEquals(infile.read(), cfile)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_5490f7ec","line":112,"updated":"2015-08-18 19:15:04.000000000","message":"when is res False?","commit_id":"aefd20882642662b4f7deeb19ddfa9617cc8c2ff"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a20455f7416ea5176823547eff8b278eeaa5e1d1","unresolved":false,"context_lines":[{"line_number":125,"context_line":"        # Remove existing and non-existing synced containers"},{"line_number":126,"context_line":"        for i in range(2):"},{"line_number":127,"context_line":"            res \u003d sds.remove_synced_container(broker)"},{"line_number":128,"context_line":"            self.assertTrue(res)"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    def test_iterate_synced_containers(self):"},{"line_number":131,"context_line":"        # populate sync container db"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_54a7974f","line":128,"updated":"2015-08-18 19:15:04.000000000","message":"again atomic, nice - but there\u0027s no test for res False?","commit_id":"aefd20882642662b4f7deeb19ddfa9617cc8c2ff"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":1,"context_line":""},{"line_number":2,"context_line":"# Copyright (c) 2010-2012 OpenStack Foundation"},{"line_number":3,"context_line":"#"},{"line_number":4,"context_line":"# Licensed under the Apache License, Version 2.0 (the \"License\");"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_55630c42","line":1,"updated":"2015-12-21 17:01:17.000000000","message":"empty line?","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":1,"context_line":""},{"line_number":2,"context_line":"# Copyright (c) 2010-2012 OpenStack Foundation"},{"line_number":3,"context_line":"#"},{"line_number":4,"context_line":"# Licensed under the Apache License, Version 2.0 (the \"License\");"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_39329811","line":1,"in_reply_to":"3a7e1126_55630c42","updated":"2016-01-01 21:44:24.000000000","message":"no longer","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":1,"context_line":""},{"line_number":2,"context_line":"# Copyright (c) 2010-2012 OpenStack Foundation"},{"line_number":3,"context_line":"#"},{"line_number":4,"context_line":"# Licensed under the Apache License, Version 2.0 (the \"License\");"},{"line_number":5,"context_line":"# you may not use this file except in compliance with the License."}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_1559048e","line":2,"updated":"2015-12-21 17:01:17.000000000","message":"2015 :) (although it could be 2016!)","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":1,"context_line":""},{"line_number":2,"context_line":"# Copyright (c) 2010-2012 OpenStack Foundation"},{"line_number":3,"context_line":"#"},{"line_number":4,"context_line":"# Licensed under the Apache License, Version 2.0 (the \"License\");"},{"line_number":5,"context_line":"# you may not use this file except in compliance with the License."}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_59350cf5","line":2,"in_reply_to":"3a7e1126_1559048e","updated":"2016-01-01 21:44:24.000000000","message":"Happy new year","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    def setUp(self):"},{"line_number":44,"context_line":"        self.logger \u003d debug_logger(\u0027test-container-sync-store\u0027)"},{"line_number":45,"context_line":"        self.logger.level \u003d logging.DEBUG"},{"line_number":46,"context_line":"        self.test_dir_prefix \u003d \u0027/tmp/test_sync_store/\u0027"},{"line_number":47,"context_line":"        if os.path.exists(self.test_dir_prefix):"},{"line_number":48,"context_line":"            rmtree(self.test_dir_prefix)"},{"line_number":49,"context_line":"        self.devices_dir \u003d os.path.join(self.test_dir_prefix, \u0027srv/node/\u0027)"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_40110c03","line":46,"updated":"2015-12-21 17:01:17.000000000","message":"use tempfile.mkdtemp()","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    def setUp(self):"},{"line_number":44,"context_line":"        self.logger \u003d debug_logger(\u0027test-container-sync-store\u0027)"},{"line_number":45,"context_line":"        self.logger.level \u003d logging.DEBUG"},{"line_number":46,"context_line":"        self.test_dir_prefix \u003d \u0027/tmp/test_sync_store/\u0027"},{"line_number":47,"context_line":"        if os.path.exists(self.test_dir_prefix):"},{"line_number":48,"context_line":"            rmtree(self.test_dir_prefix)"},{"line_number":49,"context_line":"        self.devices_dir \u003d os.path.join(self.test_dir_prefix, \u0027srv/node/\u0027)"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_1b830f58","line":46,"in_reply_to":"3a7e1126_40110c03","updated":"2016-01-01 21:44:24.000000000","message":"Done","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        self.logger.level \u003d logging.DEBUG"},{"line_number":46,"context_line":"        self.test_dir_prefix \u003d \u0027/tmp/test_sync_store/\u0027"},{"line_number":47,"context_line":"        if os.path.exists(self.test_dir_prefix):"},{"line_number":48,"context_line":"            rmtree(self.test_dir_prefix)"},{"line_number":49,"context_line":"        self.devices_dir \u003d os.path.join(self.test_dir_prefix, \u0027srv/node/\u0027)"},{"line_number":50,"context_line":"        os.makedirs(self.devices_dir)"},{"line_number":51,"context_line":"        # Create dummy container dbs"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_00f8e4c1","line":48,"updated":"2015-12-21 17:01:17.000000000","message":"i think we can trust tearDown to do this, particularly if you use mkdtemp to \"randomise\" the dir name","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        self.logger.level \u003d logging.DEBUG"},{"line_number":46,"context_line":"        self.test_dir_prefix \u003d \u0027/tmp/test_sync_store/\u0027"},{"line_number":47,"context_line":"        if os.path.exists(self.test_dir_prefix):"},{"line_number":48,"context_line":"            rmtree(self.test_dir_prefix)"},{"line_number":49,"context_line":"        self.devices_dir \u003d os.path.join(self.test_dir_prefix, \u0027srv/node/\u0027)"},{"line_number":50,"context_line":"        os.makedirs(self.devices_dir)"},{"line_number":51,"context_line":"        # Create dummy container dbs"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_3b861366","line":48,"in_reply_to":"3a7e1126_00f8e4c1","updated":"2016-01-01 21:44:24.000000000","message":"and done.","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":92,"context_line":"                container_path \u003d os.path.join(devices,"},{"line_number":93,"context_line":"                                              device,"},{"line_number":94,"context_line":"                                              DATADIR,"},{"line_number":95,"context_line":"                                              \u0027202560/218/119a.db\u0027)"},{"line_number":96,"context_line":"                self._test_path_conversions(container_path,"},{"line_number":97,"context_line":"                                            devices,"},{"line_number":98,"context_line":"                                            device,"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_a0ac708c","line":95,"updated":"2015-12-21 17:01:17.000000000","message":"nit: real container db names have 32 characters","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":92,"context_line":"                container_path \u003d os.path.join(devices,"},{"line_number":93,"context_line":"                                              device,"},{"line_number":94,"context_line":"                                              DATADIR,"},{"line_number":95,"context_line":"                                              \u0027202560/218/119a.db\u0027)"},{"line_number":96,"context_line":"                self._test_path_conversions(container_path,"},{"line_number":97,"context_line":"                                            devices,"},{"line_number":98,"context_line":"                                            device,"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_f95ac082","line":95,"in_reply_to":"3a7e1126_a0ac708c","updated":"2016-01-01 21:44:24.000000000","message":"nit but fixed","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":109,"context_line":"                                            \u0027/202560/218/119a.db\u0027)"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    def _test_path_conversions(self, path, devices, device, cpath, suffix):"},{"line_number":112,"context_line":"        sds \u003d sync_store.ContainerSyncStore(devices, self.logger, False)"},{"line_number":113,"context_line":"        if cpath \u003d\u003d sync_store.SYNC_DATADIR:"},{"line_number":114,"context_line":"            path \u003d sds._container_to_synced_container_path(path)"},{"line_number":115,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_1b3a95e7","line":112,"updated":"2015-12-21 17:01:17.000000000","message":"I\u0027d prefer the tests to be more explicit:\n\n- separate out test for each of the two conversion methods\n- define a list of input-\u003eoutput expectations\n- call method and verify output for each input\n\notherwise I have as much concern to verify the test code (below) is correct as I do the code-under-test.\n\nI learnt this principle from Clay: http://stackoverflow.com/questions/6453235/what-does-damp-not-dry-mean-when-talking-about-unit-tests","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":109,"context_line":"                                            \u0027/202560/218/119a.db\u0027)"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    def _test_path_conversions(self, path, devices, device, cpath, suffix):"},{"line_number":112,"context_line":"        sds \u003d sync_store.ContainerSyncStore(devices, self.logger, False)"},{"line_number":113,"context_line":"        if cpath \u003d\u003d sync_store.SYNC_DATADIR:"},{"line_number":114,"context_line":"            path \u003d sds._container_to_synced_container_path(path)"},{"line_number":115,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_dbfed76a","line":112,"in_reply_to":"3a7e1126_1b3a95e7","updated":"2016-01-01 21:44:24.000000000","message":"I tried the best I can to follow those principles.","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":142,"context_line":"                                            False)"},{"line_number":143,"context_line":"        cfile \u003d self.pick_dbfile()"},{"line_number":144,"context_line":"        broker \u003d FakeContainerBroker(cfile)"},{"line_number":145,"context_line":"        for i in range(2):"},{"line_number":146,"context_line":"            sds.add_synced_container(broker)"},{"line_number":147,"context_line":"            scpath \u003d sds._container_to_synced_container_path(cfile)"},{"line_number":148,"context_line":"            with open(scpath, \u0027r\u0027) as infile:"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_60bf6861","line":145,"updated":"2015-12-21 17:01:17.000000000","message":"repeated twice to verify it works when symlink already exists?","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":142,"context_line":"                                            False)"},{"line_number":143,"context_line":"        cfile \u003d self.pick_dbfile()"},{"line_number":144,"context_line":"        broker \u003d FakeContainerBroker(cfile)"},{"line_number":145,"context_line":"        for i in range(2):"},{"line_number":146,"context_line":"            sds.add_synced_container(broker)"},{"line_number":147,"context_line":"            scpath \u003d sds._container_to_synced_container_path(cfile)"},{"line_number":148,"context_line":"            with open(scpath, \u0027r\u0027) as infile:"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_fb015b6b","line":145,"in_reply_to":"3a7e1126_60bf6861","updated":"2016-01-01 21:44:24.000000000","message":"yes","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":146,"context_line":"            sds.add_synced_container(broker)"},{"line_number":147,"context_line":"            scpath \u003d sds._container_to_synced_container_path(cfile)"},{"line_number":148,"context_line":"            with open(scpath, \u0027r\u0027) as infile:"},{"line_number":149,"context_line":"                self.assertEqual(infile.read(), cfile)"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"    def test_remove_synced_container(self):"},{"line_number":152,"context_line":"        # Add a synced container to remove"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_bb17c143","line":149,"updated":"2015-12-21 17:01:17.000000000","message":"verify the generator (like with the remove test below)?","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":146,"context_line":"            sds.add_synced_container(broker)"},{"line_number":147,"context_line":"            scpath \u003d sds._container_to_synced_container_path(cfile)"},{"line_number":148,"context_line":"            with open(scpath, \u0027r\u0027) as infile:"},{"line_number":149,"context_line":"                self.assertEqual(infile.read(), cfile)"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"    def test_remove_synced_container(self):"},{"line_number":152,"context_line":"        # Add a synced container to remove"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_5bf3c79e","line":149,"in_reply_to":"3a7e1126_bb17c143","updated":"2016-01-01 21:44:24.000000000","message":"Done, How come I did not think about this myself :-)","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":165,"context_line":"        for db_path in sds.synced_containers_generator():"},{"line_number":166,"context_line":"            iterated_synced_containers.append(db_path)"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"        self.assertEqual(len(iterated_synced_containers), 0)"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"    def test_remove_synced_container_links(self):"},{"line_number":171,"context_line":"        # Add a synced container to remove"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_601be881","line":168,"updated":"2015-12-21 17:01:17.000000000","message":"or\n\n  self.assertFalse(iterated_synced_containers)\n\nbecause if it fails it will print the content of the list","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":165,"context_line":"        for db_path in sds.synced_containers_generator():"},{"line_number":166,"context_line":"            iterated_synced_containers.append(db_path)"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"        self.assertEqual(len(iterated_synced_containers), 0)"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"    def test_remove_synced_container_links(self):"},{"line_number":171,"context_line":"        # Add a synced container to remove"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_5b2187fa","line":168,"in_reply_to":"3a7e1126_601be881","updated":"2016-01-01 21:44:24.000000000","message":"For the sake of symmetry with the add_synced_containers I left it as is","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":168,"context_line":"        self.assertEqual(len(iterated_synced_containers), 0)"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"    def test_remove_synced_container_links(self):"},{"line_number":171,"context_line":"        # Add a synced container to remove"},{"line_number":172,"context_line":"        sds \u003d sync_store.ContainerSyncStore(self.devices_dir,"},{"line_number":173,"context_line":"                                            self.logger,"},{"line_number":174,"context_line":"                                            False)"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_7b539917","line":171,"updated":"2015-12-21 17:01:17.000000000","message":"can this test be merged with previous - it seems to only add the verification of all path parts being deleted","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":168,"context_line":"        self.assertEqual(len(iterated_synced_containers), 0)"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"    def test_remove_synced_container_links(self):"},{"line_number":171,"context_line":"        # Add a synced container to remove"},{"line_number":172,"context_line":"        sds \u003d sync_store.ContainerSyncStore(self.devices_dir,"},{"line_number":173,"context_line":"                                            self.logger,"},{"line_number":174,"context_line":"                                            False)"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_fb0f1b86","line":171,"in_reply_to":"3a7e1126_7b539917","updated":"2016-01-01 21:44:24.000000000","message":"Absolutely!","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":189,"context_line":"        self.assertEqual(len(iterated_synced_containers), 0)"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        # Make sure the whole link path gets deleted"},{"line_number":192,"context_line":"        self.assertTrue(not os.path.exists(lfile))"},{"line_number":193,"context_line":"        lfile \u003d os.path.dirname(lfile)"},{"line_number":194,"context_line":"        self.assertTrue(not os.path.exists(os.path.dirname(lfile)))"},{"line_number":195,"context_line":"        cfile \u003d os.path.dirname(lfile)"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_7b887971","line":192,"updated":"2015-12-21 17:01:17.000000000","message":"use assertFalse instead of assertTrue(not ...)","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":189,"context_line":"        self.assertEqual(len(iterated_synced_containers), 0)"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        # Make sure the whole link path gets deleted"},{"line_number":192,"context_line":"        self.assertTrue(not os.path.exists(lfile))"},{"line_number":193,"context_line":"        lfile \u003d os.path.dirname(lfile)"},{"line_number":194,"context_line":"        self.assertTrue(not os.path.exists(os.path.dirname(lfile)))"},{"line_number":195,"context_line":"        cfile \u003d os.path.dirname(lfile)"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_3b7db3e7","line":192,"in_reply_to":"3a7e1126_7b887971","updated":"2016-01-01 21:44:24.000000000","message":"Done.","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":192,"context_line":"        self.assertTrue(not os.path.exists(lfile))"},{"line_number":193,"context_line":"        lfile \u003d os.path.dirname(lfile)"},{"line_number":194,"context_line":"        self.assertTrue(not os.path.exists(os.path.dirname(lfile)))"},{"line_number":195,"context_line":"        cfile \u003d os.path.dirname(lfile)"},{"line_number":196,"context_line":"        self.assertTrue(not os.path.exists(os.path.dirname(lfile)))"},{"line_number":197,"context_line":"        lfile \u003d os.path.dirname(lfile)"},{"line_number":198,"context_line":"        self.assertTrue(not os.path.exists(os.path.dirname(lfile)))"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_1bf29506","line":195,"updated":"2015-12-21 17:01:17.000000000","message":"cfile is not used","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":192,"context_line":"        self.assertTrue(not os.path.exists(lfile))"},{"line_number":193,"context_line":"        lfile \u003d os.path.dirname(lfile)"},{"line_number":194,"context_line":"        self.assertTrue(not os.path.exists(os.path.dirname(lfile)))"},{"line_number":195,"context_line":"        cfile \u003d os.path.dirname(lfile)"},{"line_number":196,"context_line":"        self.assertTrue(not os.path.exists(os.path.dirname(lfile)))"},{"line_number":197,"context_line":"        lfile \u003d os.path.dirname(lfile)"},{"line_number":198,"context_line":"        self.assertTrue(not os.path.exists(os.path.dirname(lfile)))"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_db8337cb","line":195,"in_reply_to":"3a7e1126_1bf29506","updated":"2016-01-01 21:44:24.000000000","message":"Function no longer exists :-)","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":216,"context_line":"            iterated_synced_containers.append(db_path)"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"        self.assertTrue("},{"line_number":219,"context_line":"            set(containers) \u003d\u003d set(iterated_synced_containers))"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"    def test_unhandled_exceptions_in_add_remove(self):"},{"line_number":222,"context_line":"        def fake_os_call(var1, var2\u003dNone):"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_5b9eddb9","line":219,"updated":"2015-12-21 17:01:17.000000000","message":"use assertEqual","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":216,"context_line":"            iterated_synced_containers.append(db_path)"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"        self.assertTrue("},{"line_number":219,"context_line":"            set(containers) \u003d\u003d set(iterated_synced_containers))"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"    def test_unhandled_exceptions_in_add_remove(self):"},{"line_number":222,"context_line":"        def fake_os_call(var1, var2\u003dNone):"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_fb863bda","line":219,"in_reply_to":"3a7e1126_5b9eddb9","updated":"2016-01-01 21:44:24.000000000","message":"yep","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":235,"context_line":"        sync_store.os.stat \u003d fake_os_call"},{"line_number":236,"context_line":"        try:"},{"line_number":237,"context_line":"            sds.add_synced_container(broker)"},{"line_number":238,"context_line":"        except OSError as oserr:"},{"line_number":239,"context_line":"            self.assertEqual(oserr.errno, errno.EPERM)"},{"line_number":240,"context_line":"        finally:"},{"line_number":241,"context_line":"            sync_store.os.stat \u003d orig_stat"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_9b80c58c","line":238,"updated":"2015-12-21 17:01:17.000000000","message":"does not fail if the exception is not raised - see my diff","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":235,"context_line":"        sync_store.os.stat \u003d fake_os_call"},{"line_number":236,"context_line":"        try:"},{"line_number":237,"context_line":"            sds.add_synced_container(broker)"},{"line_number":238,"context_line":"        except OSError as oserr:"},{"line_number":239,"context_line":"            self.assertEqual(oserr.errno, errno.EPERM)"},{"line_number":240,"context_line":"        finally:"},{"line_number":241,"context_line":"            sync_store.os.stat \u003d orig_stat"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_794fb0bd","line":238,"in_reply_to":"3a7e1126_9b80c58c","updated":"2016-01-01 21:44:24.000000000","message":"yet another tests eye opener for me :-)","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3791f27cfe0b36a5145140d9c34955aaaf98728","unresolved":false,"context_lines":[{"line_number":264,"context_line":"        finally:"},{"line_number":265,"context_line":"            sync_store.os.unlink \u003d orig_unlink"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"    def test_update_sync_store(self):"},{"line_number":268,"context_line":"        add_sync \u003d list()"},{"line_number":269,"context_line":"        remove_sync \u003d list()"},{"line_number":270,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"3a7e1126_bb526133","line":267,"updated":"2015-12-21 17:01:17.000000000","message":"see my diff for test_replicator for how these tests could be written using mock.patch for the add_ and remove_ methods.\n\n(this test is very similar to the one in test_replicator)","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"51bfdd1935e0c89a4fb920112d67b08f5c500412","unresolved":false,"context_lines":[{"line_number":264,"context_line":"        finally:"},{"line_number":265,"context_line":"            sync_store.os.unlink \u003d orig_unlink"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"    def test_update_sync_store(self):"},{"line_number":268,"context_line":"        add_sync \u003d list()"},{"line_number":269,"context_line":"        remove_sync \u003d list()"},{"line_number":270,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_7959f059","line":267,"in_reply_to":"3a7e1126_bb526133","updated":"2016-01-01 21:44:24.000000000","message":"Fixed. and agree this is similar, only perhaps more comprehensive.","commit_id":"978b8c7086327e97f06300a1a25048a2f4105339"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4b85b8ceb32be975e46a15dbe0325c352af16e1e","unresolved":false,"context_lines":[{"line_number":44,"context_line":"    def setUp(self):"},{"line_number":45,"context_line":"        self.logger \u003d debug_logger(\u0027test-container-sync-store\u0027)"},{"line_number":46,"context_line":"        self.logger.level \u003d logging.DEBUG"},{"line_number":47,"context_line":"        self.test_dir_prefix \u003d tempfile.mkdtemp(prefix\u003d\u0027/tmp/\u0027)"},{"line_number":48,"context_line":"        self.devices_dir \u003d os.path.join(self.test_dir_prefix, \u0027srv/node/\u0027)"},{"line_number":49,"context_line":"        os.makedirs(self.devices_dir)"},{"line_number":50,"context_line":"        # Create dummy container dbs"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_b0407c35","line":47,"updated":"2016-01-05 16:58:35.000000000","message":"tempfile.mkdtemp() is sufficient to create /tmp/randomdirname","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"1894a6f77a2cee4c903ef90cbea78e7e8b593469","unresolved":false,"context_lines":[{"line_number":44,"context_line":"    def setUp(self):"},{"line_number":45,"context_line":"        self.logger \u003d debug_logger(\u0027test-container-sync-store\u0027)"},{"line_number":46,"context_line":"        self.logger.level \u003d logging.DEBUG"},{"line_number":47,"context_line":"        self.test_dir_prefix \u003d tempfile.mkdtemp(prefix\u003d\u0027/tmp/\u0027)"},{"line_number":48,"context_line":"        self.devices_dir \u003d os.path.join(self.test_dir_prefix, \u0027srv/node/\u0027)"},{"line_number":49,"context_line":"        os.makedirs(self.devices_dir)"},{"line_number":50,"context_line":"        # Create dummy container dbs"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_a7b4b634","line":47,"in_reply_to":"fa69d971_b0407c35","updated":"2016-01-05 21:23:33.000000000","message":"got rid of the prefix","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4b85b8ceb32be975e46a15dbe0325c352af16e1e","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                            hsh,"},{"line_number":83,"context_line":"                            \u0027%s.db\u0027 % hsh)"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"    ###"},{"line_number":86,"context_line":"    # Path converison tests"},{"line_number":87,"context_line":"    # container path is of the form:"},{"line_number":88,"context_line":"    # /srv/node/sdb/containers/part/.../*.db"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_30c1ec54","line":85,"updated":"2016-01-05 16:58:35.000000000","message":"delete the ###","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"1894a6f77a2cee4c903ef90cbea78e7e8b593469","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                            hsh,"},{"line_number":83,"context_line":"                            \u0027%s.db\u0027 % hsh)"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"    ###"},{"line_number":86,"context_line":"    # Path converison tests"},{"line_number":87,"context_line":"    # container path is of the form:"},{"line_number":88,"context_line":"    # /srv/node/sdb/containers/part/.../*.db"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_67f0fefe","line":85,"in_reply_to":"fa69d971_30c1ec54","updated":"2016-01-05 21:23:33.000000000","message":"Done","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4b85b8ceb32be975e46a15dbe0325c352af16e1e","unresolved":false,"context_lines":[{"line_number":83,"context_line":"                            \u0027%s.db\u0027 % hsh)"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"    ###"},{"line_number":86,"context_line":"    # Path converison tests"},{"line_number":87,"context_line":"    # container path is of the form:"},{"line_number":88,"context_line":"    # /srv/node/sdb/containers/part/.../*.db"},{"line_number":89,"context_line":"    # or more generally:"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_256a0cef","line":86,"updated":"2016-01-05 16:58:35.000000000","message":"nit s/converison/conversion/","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"1894a6f77a2cee4c903ef90cbea78e7e8b593469","unresolved":false,"context_lines":[{"line_number":83,"context_line":"                            \u0027%s.db\u0027 % hsh)"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"    ###"},{"line_number":86,"context_line":"    # Path converison tests"},{"line_number":87,"context_line":"    # container path is of the form:"},{"line_number":88,"context_line":"    # /srv/node/sdb/containers/part/.../*.db"},{"line_number":89,"context_line":"    # or more generally:"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_c7df6a67","line":86,"in_reply_to":"fa69d971_256a0cef","updated":"2016-01-05 21:23:33.000000000","message":"Done","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4b85b8ceb32be975e46a15dbe0325c352af16e1e","unresolved":false,"context_lines":[{"line_number":147,"context_line":"    def _db_path_suffix(self):"},{"line_number":148,"context_line":"        def random_hexa_string(length):"},{"line_number":149,"context_line":"            hexa_chars \u003d \u00271234567890abcdef\u0027"},{"line_number":150,"context_line":"            return \u0027\u0027.join([random.choice(hexa_chars) for x in range(length)])"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"        db \u003d random_hexa_string(32)"},{"line_number":153,"context_line":"        return \u0027%s/%s/%s/%s.db\u0027 % (random_hexa_string(5),"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_f0ea7473","line":150,"updated":"2016-01-05 16:58:35.000000000","message":"or\n\n  \u0027%lengthx\u0027 % random.randrange(16**length)\n\nhttp://stackoverflow.com/questions/2782229/most-lightweight-way-to-create-a-random-string-and-a-random-hexadecimal-number","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"1894a6f77a2cee4c903ef90cbea78e7e8b593469","unresolved":false,"context_lines":[{"line_number":147,"context_line":"    def _db_path_suffix(self):"},{"line_number":148,"context_line":"        def random_hexa_string(length):"},{"line_number":149,"context_line":"            hexa_chars \u003d \u00271234567890abcdef\u0027"},{"line_number":150,"context_line":"            return \u0027\u0027.join([random.choice(hexa_chars) for x in range(length)])"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"        db \u003d random_hexa_string(32)"},{"line_number":153,"context_line":"        return \u0027%s/%s/%s/%s.db\u0027 % (random_hexa_string(5),"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_a7a83678","line":150,"in_reply_to":"fa69d971_f0ea7473","updated":"2016-01-05 21:23:33.000000000","message":"you mean \u0027%0xlength\u0027 % random.randrange(16**length) :-)","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4b85b8ceb32be975e46a15dbe0325c352af16e1e","unresolved":false,"context_lines":[{"line_number":186,"context_line":"        # Validate path against \u0027devices\u0027"},{"line_number":187,"context_line":"        for p in devices.split(\u0027/\u0027):"},{"line_number":188,"context_line":"            if p \u003d\u003d \u0027\u0027:"},{"line_number":189,"context_line":"                continue"},{"line_number":190,"context_line":"            self.assertEqual(spath[i], p)"},{"line_number":191,"context_line":"            i \u003d i + 1"},{"line_number":192,"context_line":"        # Validate path against \u0027device\u0027"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_eb84c50b","line":189,"updated":"2016-01-05 16:58:35.000000000","message":"for p in [p for p in devices.split(\u0027/\u0027) if p]:","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"1894a6f77a2cee4c903ef90cbea78e7e8b593469","unresolved":false,"context_lines":[{"line_number":186,"context_line":"        # Validate path against \u0027devices\u0027"},{"line_number":187,"context_line":"        for p in devices.split(\u0027/\u0027):"},{"line_number":188,"context_line":"            if p \u003d\u003d \u0027\u0027:"},{"line_number":189,"context_line":"                continue"},{"line_number":190,"context_line":"            self.assertEqual(spath[i], p)"},{"line_number":191,"context_line":"            i \u003d i + 1"},{"line_number":192,"context_line":"        # Validate path against \u0027device\u0027"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_4a6061a5","line":189,"in_reply_to":"fa69d971_eb84c50b","updated":"2016-01-05 21:23:33.000000000","message":"Nice","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4b85b8ceb32be975e46a15dbe0325c352af16e1e","unresolved":false,"context_lines":[{"line_number":188,"context_line":"            if p \u003d\u003d \u0027\u0027:"},{"line_number":189,"context_line":"                continue"},{"line_number":190,"context_line":"            self.assertEqual(spath[i], p)"},{"line_number":191,"context_line":"            i \u003d i + 1"},{"line_number":192,"context_line":"        # Validate path against \u0027device\u0027"},{"line_number":193,"context_line":"        for p in device.split(\u0027/\u0027):"},{"line_number":194,"context_line":"            if p \u003d\u003d \u0027\u0027:"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_0b911947","line":191,"updated":"2016-01-05 16:58:35.000000000","message":"simpler to use list pop() instead of maintaining the index - see diff","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"1894a6f77a2cee4c903ef90cbea78e7e8b593469","unresolved":false,"context_lines":[{"line_number":188,"context_line":"            if p \u003d\u003d \u0027\u0027:"},{"line_number":189,"context_line":"                continue"},{"line_number":190,"context_line":"            self.assertEqual(spath[i], p)"},{"line_number":191,"context_line":"            i \u003d i + 1"},{"line_number":192,"context_line":"        # Validate path against \u0027device\u0027"},{"line_number":193,"context_line":"        for p in device.split(\u0027/\u0027):"},{"line_number":194,"context_line":"            if p \u003d\u003d \u0027\u0027:"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_67c31eee","line":191,"in_reply_to":"fa69d971_0b911947","updated":"2016-01-05 21:23:33.000000000","message":"Nice!","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4b85b8ceb32be975e46a15dbe0325c352af16e1e","unresolved":false,"context_lines":[{"line_number":249,"context_line":"        self.assertFalse(os.path.exists(lfile))"},{"line_number":250,"context_line":"        lfile \u003d os.path.dirname(lfile)"},{"line_number":251,"context_line":"        self.assertFalse(os.path.exists(os.path.dirname(lfile)))"},{"line_number":252,"context_line":"        cfile \u003d os.path.dirname(lfile)"},{"line_number":253,"context_line":"        self.assertFalse(os.path.exists(os.path.dirname(lfile)))"},{"line_number":254,"context_line":"        lfile \u003d os.path.dirname(lfile)"},{"line_number":255,"context_line":"        self.assertFalse(os.path.exists(os.path.dirname(lfile)))"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_3d1544e4","line":252,"updated":"2016-01-05 16:58:35.000000000","message":"cfile not used","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"1894a6f77a2cee4c903ef90cbea78e7e8b593469","unresolved":false,"context_lines":[{"line_number":249,"context_line":"        self.assertFalse(os.path.exists(lfile))"},{"line_number":250,"context_line":"        lfile \u003d os.path.dirname(lfile)"},{"line_number":251,"context_line":"        self.assertFalse(os.path.exists(os.path.dirname(lfile)))"},{"line_number":252,"context_line":"        cfile \u003d os.path.dirname(lfile)"},{"line_number":253,"context_line":"        self.assertFalse(os.path.exists(os.path.dirname(lfile)))"},{"line_number":254,"context_line":"        lfile \u003d os.path.dirname(lfile)"},{"line_number":255,"context_line":"        self.assertFalse(os.path.exists(os.path.dirname(lfile)))"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_ca6a11b3","line":252,"in_reply_to":"fa69d971_3d1544e4","updated":"2016-01-05 21:23:33.000000000","message":"Done","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4b85b8ceb32be975e46a15dbe0325c352af16e1e","unresolved":false,"context_lines":[{"line_number":316,"context_line":"        # sync-to and sync-key metadata items and whether"},{"line_number":317,"context_line":"        # the database is marked for delete."},{"line_number":318,"context_line":"        # The table below summarizes the expected result"},{"line_number":319,"context_line":"        # for the varios combinations, e.g.:"},{"line_number":320,"context_line":"        # If metadata items exist and not and the database"},{"line_number":321,"context_line":"        # is not marked for delete then add should be called."},{"line_number":322,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_fd8ecc2c","line":319,"updated":"2016-01-05 16:58:35.000000000","message":"s/varios/various/","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"1894a6f77a2cee4c903ef90cbea78e7e8b593469","unresolved":false,"context_lines":[{"line_number":316,"context_line":"        # sync-to and sync-key metadata items and whether"},{"line_number":317,"context_line":"        # the database is marked for delete."},{"line_number":318,"context_line":"        # The table below summarizes the expected result"},{"line_number":319,"context_line":"        # for the varios combinations, e.g.:"},{"line_number":320,"context_line":"        # If metadata items exist and not and the database"},{"line_number":321,"context_line":"        # is not marked for delete then add should be called."},{"line_number":322,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_6aa205ff","line":319,"in_reply_to":"fa69d971_fd8ecc2c","updated":"2016-01-05 21:23:33.000000000","message":"Done","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4b85b8ceb32be975e46a15dbe0325c352af16e1e","unresolved":false,"context_lines":[{"line_number":317,"context_line":"        # the database is marked for delete."},{"line_number":318,"context_line":"        # The table below summarizes the expected result"},{"line_number":319,"context_line":"        # for the varios combinations, e.g.:"},{"line_number":320,"context_line":"        # If metadata items exist and not and the database"},{"line_number":321,"context_line":"        # is not marked for delete then add should be called."},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"        results_list \u003d ["}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_5da5989f","line":320,"updated":"2016-01-05 16:58:35.000000000","message":"typo \u0027and not\u0027 ?","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"1894a6f77a2cee4c903ef90cbea78e7e8b593469","unresolved":false,"context_lines":[{"line_number":317,"context_line":"        # the database is marked for delete."},{"line_number":318,"context_line":"        # The table below summarizes the expected result"},{"line_number":319,"context_line":"        # for the varios combinations, e.g.:"},{"line_number":320,"context_line":"        # If metadata items exist and not and the database"},{"line_number":321,"context_line":"        # is not marked for delete then add should be called."},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"        results_list \u003d ["}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_2aa88dda","line":320,"in_reply_to":"fa69d971_5da5989f","updated":"2016-01-05 21:23:33.000000000","message":"Done","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4b85b8ceb32be975e46a15dbe0325c352af16e1e","unresolved":false,"context_lines":[{"line_number":353,"context_line":"                # We now iterate over the list of combinations"},{"line_number":354,"context_line":"                # Validating that add and removed are called as"},{"line_number":355,"context_line":"                # expected"},{"line_number":356,"context_line":"                for result in results_list:"},{"line_number":357,"context_line":"                    cfile \u003d self.pick_dbfile()"},{"line_number":358,"context_line":"                    broker \u003d FakeContainerBroker(cfile)"},{"line_number":359,"context_line":"                    if result[0] is True:"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_bda13448","line":356,"updated":"2016-01-05 16:58:35.000000000","message":"easier to read the code if we cast result to a tuple of variables: deleted, sync_to, sync_key, expected\n\nsee diff","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"1894a6f77a2cee4c903ef90cbea78e7e8b593469","unresolved":false,"context_lines":[{"line_number":353,"context_line":"                # We now iterate over the list of combinations"},{"line_number":354,"context_line":"                # Validating that add and removed are called as"},{"line_number":355,"context_line":"                # expected"},{"line_number":356,"context_line":"                for result in results_list:"},{"line_number":357,"context_line":"                    cfile \u003d self.pick_dbfile()"},{"line_number":358,"context_line":"                    broker \u003d FakeContainerBroker(cfile)"},{"line_number":359,"context_line":"                    if result[0] is True:"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa69d971_aab9dd16","line":356,"in_reply_to":"fa69d971_bda13448","updated":"2016-01-05 21:23:33.000000000","message":"Nice.","commit_id":"052b4061f4b2ab0b6339a6726f4654d472cc3bf2"},{"author":{"_account_id":330,"name":"John Dickinson","email":"me@not.mn","username":"notmyname"},"change_message_id":"0b70c79117d7fd842b302ce64ad3fcbf29968f0b","unresolved":false,"context_lines":[{"line_number":145,"context_line":"    # 133791/625/82...25/82...25.db"},{"line_number":146,"context_line":"    def _db_path_suffix(self):"},{"line_number":147,"context_line":"        def random_hexa_string(length):"},{"line_number":148,"context_line":"            \u0027%0xlength\u0027 % random.randrange(16 ** length)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"        db \u003d random_hexa_string(32)"},{"line_number":151,"context_line":"        return \u0027%s/%s/%s/%s.db\u0027 % (random_hexa_string(5),"}],"source_content_type":"text/x-python","patch_set":31,"id":"9a68dd71_6dd87f1e","line":148,"updated":"2016-01-20 17:29:29.000000000","message":"This doesn\u0027t look right. It should be fixed, but I\u0027m not sure it actually matters for the test.","commit_id":"85a0a6a28e166bc076cf8786de2b46248d8786a2"}]}