)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0f65be2c07a6c64d39e90e5c5454dcb44f3c7ddf","unresolved":false,"context_lines":[{"line_number":16,"context_line":"to keep track of sync statistics. This, again, follows the container"},{"line_number":17,"context_line":"updater pid2filename usage only with pipes instead of files."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"The saio config files were edited to facilitate the probe tests,"},{"line_number":20,"context_line":"and the default for concurrency is 1 to best maintain current"},{"line_number":21,"context_line":"run time behavior.\\"},{"line_number":22,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"da20952f_a6ed41b9","line":19,"updated":"2015-09-08 18:36:55.000000000","message":"Can you elaborate?  What is it about the probetests that require the config change in a swift-all-in-one - do they fail?  Is there anyway to make those tests pass with the default config options - or detect when the concurrency isn\u0027t \"at least 4\" and Skip them?","commit_id":"ee486b4ac60fe8e5f54373031856f8f71330d2be"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"d13015d18bc1748d5ce16b0b8e9c4347331835cc","unresolved":false,"context_lines":[{"line_number":16,"context_line":"to keep track of sync statistics. This, again, follows the container"},{"line_number":17,"context_line":"updater pid2filename usage only with pipes instead of files."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"The saio config files were edited to facilitate the probe tests,"},{"line_number":20,"context_line":"and the default for concurrency is 1 to best maintain current"},{"line_number":21,"context_line":"run time behavior.\\"},{"line_number":22,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"ba15a1d1_0d9a2115","line":19,"in_reply_to":"da20952f_a6ed41b9","updated":"2015-09-12 21:29:53.000000000","message":"On one hand I want to keep the defaults to a concurrency (processes) of 1.\nOn the other to do meaningful testing I wanted \u003e1 processes.\nAny better way of doing so without altering the saio conf files?","commit_id":"ee486b4ac60fe8e5f54373031856f8f71330d2be"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0f65be2c07a6c64d39e90e5c5454dcb44f3c7ddf","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"The saio config files were edited to facilitate the probe tests,"},{"line_number":20,"context_line":"and the default for concurrency is 1 to best maintain current"},{"line_number":21,"context_line":"run time behavior.\\"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: I8566b8b0d5511e93f8f5ea2eb2a25d0d3631bae8"},{"line_number":24,"context_line":"Partial-Bug: #1068426"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"da20952f_86c6fd66","line":21,"updated":"2015-09-08 18:36:55.000000000","message":"\" \\ \" ???","commit_id":"ee486b4ac60fe8e5f54373031856f8f71330d2be"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"d13015d18bc1748d5ce16b0b8e9c4347331835cc","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"The saio config files were edited to facilitate the probe tests,"},{"line_number":20,"context_line":"and the default for concurrency is 1 to best maintain current"},{"line_number":21,"context_line":"run time behavior.\\"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: I8566b8b0d5511e93f8f5ea2eb2a25d0d3631bae8"},{"line_number":24,"context_line":"Partial-Bug: #1068426"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"ba15a1d1_ed948de9","line":21,"in_reply_to":"da20952f_86c6fd66","updated":"2015-09-12 21:29:53.000000000","message":"That was redundant. removed.","commit_id":"ee486b4ac60fe8e5f54373031856f8f71330d2be"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e4a7e350a9c415f16824e51020420de29ffec3f0","unresolved":false,"context_lines":[{"line_number":17,"context_line":"updater pid2filename usage only with pipes instead of files."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"The default for processes is set to 1 so as to maintain current"},{"line_number":20,"context_line":"run time behavior."},{"line_number":21,"context_line":"The saio config files were edited to facilitate the probe tests"},{"line_number":22,"context_line":"so as to test the daemon with \u003e 1 processes."},{"line_number":23,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":21,"id":"dada55a8_f8e12f2b","line":20,"updated":"2016-07-21 12:20:36.000000000","message":"this isn\u0027t strictly true because even with the default setting or processes\u003d\u003d1 an extra process is spawned with this patch.","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"505e100dd79874025a6d6ffcf516fc6a4f3ecac1","unresolved":false,"context_lines":[{"line_number":19,"context_line":"The default for processes is set to 1 so as to maintain current"},{"line_number":20,"context_line":"run time behavior."},{"line_number":21,"context_line":"The saio config files were edited to facilitate the probe tests"},{"line_number":22,"context_line":"so as to test the daemon with \u003e 1 processes."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"This patch is closely related to [1] which adds thread level"},{"line_number":25,"context_line":"concurrency to the sync daemon. [2] gives full details on"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":28,"id":"9ad45d7e_1724d7c2","line":22,"updated":"2016-08-12 17:18:07.000000000","message":"I think the SAIO config should reflect the default. Devs can deviate from that to test non-default configs.","commit_id":"beffc5014c01ee62cecefe447122912129418155"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"b2d0923c9b7e6b2d377c7da5ea5b7722dc242123","unresolved":false,"context_lines":[{"line_number":19,"context_line":"The default for processes is set to 1 so as to maintain current"},{"line_number":20,"context_line":"run time behavior."},{"line_number":21,"context_line":"The saio config files were edited to facilitate the probe tests"},{"line_number":22,"context_line":"so as to test the daemon with \u003e 1 processes."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"This patch is closely related to [1] which adds thread level"},{"line_number":25,"context_line":"concurrency to the sync daemon. [2] gives full details on"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":28,"id":"3ac371cc_59b64f3e","line":22,"in_reply_to":"9ad45d7e_1724d7c2","updated":"2016-08-15 10:55:59.000000000","message":"Yes, this makes sense to me","commit_id":"beffc5014c01ee62cecefe447122912129418155"}],"doc/saio/swift/container-server/1.conf":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"505e100dd79874025a6d6ffcf516fc6a4f3ecac1","unresolved":false,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"[container-sync]"},{"line_number":30,"context_line":"interval \u003d 10"},{"line_number":31,"context_line":"processes \u003d 4"}],"source_content_type":"text/plain","patch_set":28,"id":"9ad45d7e_52181d75","line":31,"updated":"2016-08-12 17:18:07.000000000","message":"see note in commit message. I think that SAIO should use the defaults, so no changes in this and other conf files.","commit_id":"beffc5014c01ee62cecefe447122912129418155"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"b2d0923c9b7e6b2d377c7da5ea5b7722dc242123","unresolved":false,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"[container-sync]"},{"line_number":30,"context_line":"interval \u003d 10"},{"line_number":31,"context_line":"processes \u003d 4"}],"source_content_type":"text/plain","patch_set":28,"id":"3ac371cc_d9807f49","line":31,"in_reply_to":"9ad45d7e_52181d75","updated":"2016-08-15 10:55:59.000000000","message":"Done","commit_id":"beffc5014c01ee62cecefe447122912129418155"}],"etc/container-server.conf-sample":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0f65be2c07a6c64d39e90e5c5454dcb44f3c7ddf","unresolved":false,"context_lines":[{"line_number":182,"context_line":"# internal_client_conf_path \u003d /etc/swift/internal-client.conf"},{"line_number":183,"context_line":"#"},{"line_number":184,"context_line":"# The maxumum number of sync processes running in parallel"},{"line_number":185,"context_line":"# concurrency \u003d 1"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"# Note: Put it at the beginning of the pipeline to profile all middleware. But"},{"line_number":188,"context_line":"# it is safer to put this after healthcheck."}],"source_content_type":"application/octet-stream","patch_set":3,"id":"da20952f_061e6d5f","line":185,"updated":"2015-09-08 18:36:55.000000000","message":"It looks like at least the object.updater uses concurrency to mean number of processes - but I think it\u0027s more common to see it used to describe # of greenthreads.\n\nI think there\u0027s a good chance with all the network ops in this daemon that having greened concurrency would *also* be good (maybe more bang for the buck really - i\u0027m not sure).  But even if process level concurrency is a higher priority we should be mindful of wanting *both* at some point.\n\n... with that in mind I think \"processes\" and \"concurrency\" might both be good tunables to have - or \"workers\" and maybe \"pool_size\" at some point or something?","commit_id":"ee486b4ac60fe8e5f54373031856f8f71330d2be"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a8bf2a5614904d345f77ddc65a31b3692de62f41","unresolved":false,"context_lines":[{"line_number":182,"context_line":"# internal_client_conf_path \u003d /etc/swift/internal-client.conf"},{"line_number":183,"context_line":"#"},{"line_number":184,"context_line":"# The maxumum number of sync processes running in parallel"},{"line_number":185,"context_line":"# concurrency \u003d 1"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"# Note: Put it at the beginning of the pipeline to profile all middleware. But"},{"line_number":188,"context_line":"# it is safer to put this after healthcheck."}],"source_content_type":"application/octet-stream","patch_set":3,"id":"ba15a1d1_da89a876","line":185,"in_reply_to":"ba15a1d1_ad9e9505","updated":"2015-09-16 20:10:00.000000000","message":"Thanks makes sense.  I\u0027m sure threads in this context is greenthreads - os threads (threading module) suck in python.","commit_id":"ee486b4ac60fe8e5f54373031856f8f71330d2be"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"d13015d18bc1748d5ce16b0b8e9c4347331835cc","unresolved":false,"context_lines":[{"line_number":182,"context_line":"# internal_client_conf_path \u003d /etc/swift/internal-client.conf"},{"line_number":183,"context_line":"#"},{"line_number":184,"context_line":"# The maxumum number of sync processes running in parallel"},{"line_number":185,"context_line":"# concurrency \u003d 1"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"# Note: Put it at the beginning of the pipeline to profile all middleware. But"},{"line_number":188,"context_line":"# it is safer to put this after healthcheck."}],"source_content_type":"application/octet-stream","patch_set":3,"id":"ba15a1d1_ad9e9505","line":185,"in_reply_to":"da20952f_061e6d5f","updated":"2015-09-12 21:29:53.000000000","message":"s/concurrency/processes\nOtherwise, indeed the plan is to be able to tune both processes and threads - I am just breaking this into two patch sets so as to ease on the review process.\n\nIn fact we have numbers that pretty much show the following:\nprocesses \u003d 1, threads \u003d 1: container sync worth of BW is say X Mb/sec\nprocesses \u003d 10, threads \u003d 1: container sync worth of BW is 10X Mb/sec\nprocesses \u003d 1, threads \u003d 10: container sync worth of BW is 10X Mb/sec\nprocesses \u003d 10, threads \u003d 10: container sync worth of BW is 100X Mb/sec\n\n\"Architecure-wise\" different processes deal with different containers, and different threads will deal with different entries in the container listing table.","commit_id":"ee486b4ac60fe8e5f54373031856f8f71330d2be"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e4a7e350a9c415f16824e51020420de29ffec3f0","unresolved":false,"context_lines":[{"line_number":195,"context_line":"# Internal client config file path"},{"line_number":196,"context_line":"# internal_client_conf_path \u003d /etc/swift/internal-client.conf"},{"line_number":197,"context_line":"#"},{"line_number":198,"context_line":"# The maxumum number of sync processes running in parallel"},{"line_number":199,"context_line":"# processes \u003d 1"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"# Note: Put it at the beginning of the pipeline to profile all middleware. But"}],"source_content_type":"application/octet-stream","patch_set":21,"id":"dada55a8_58503b0b","line":198,"updated":"2016-07-21 12:20:36.000000000","message":"s/maxumum/maximum/\n\nplease also state the default value is 1","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"4f74852fad09fa761891a9372395c6639121ec9e","unresolved":false,"context_lines":[{"line_number":195,"context_line":"# Internal client config file path"},{"line_number":196,"context_line":"# internal_client_conf_path \u003d /etc/swift/internal-client.conf"},{"line_number":197,"context_line":"#"},{"line_number":198,"context_line":"# The maxumum number of sync processes running in parallel"},{"line_number":199,"context_line":"# processes \u003d 1"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"# Note: Put it at the beginning of the pipeline to profile all middleware. But"}],"source_content_type":"application/octet-stream","patch_set":21,"id":"bacf61ea_aa98081d","line":198,"in_reply_to":"dada55a8_58503b0b","updated":"2016-08-01 13:47:20.000000000","message":"Done","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"505e100dd79874025a6d6ffcf516fc6a4f3ecac1","unresolved":false,"context_lines":[{"line_number":196,"context_line":"# Internal client config file path"},{"line_number":197,"context_line":"# internal_client_conf_path \u003d /etc/swift/internal-client.conf"},{"line_number":198,"context_line":"#"},{"line_number":199,"context_line":"# The maximum number of sync processes running in parallel (default\u003d1)"},{"line_number":200,"context_line":"# processes \u003d 1"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"# Note: Put it at the beginning of the pipeline to profile all middleware. But"}],"source_content_type":"application/octet-stream","patch_set":28,"id":"9ad45d7e_b285195b","line":199,"updated":"2016-08-12 17:18:07.000000000","message":"nit: perhaps...\n\n  The maximum number of child sync processes to run in parallel (default\u003d1)","commit_id":"beffc5014c01ee62cecefe447122912129418155"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"b2d0923c9b7e6b2d377c7da5ea5b7722dc242123","unresolved":false,"context_lines":[{"line_number":196,"context_line":"# Internal client config file path"},{"line_number":197,"context_line":"# internal_client_conf_path \u003d /etc/swift/internal-client.conf"},{"line_number":198,"context_line":"#"},{"line_number":199,"context_line":"# The maximum number of sync processes running in parallel (default\u003d1)"},{"line_number":200,"context_line":"# processes \u003d 1"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"# Note: Put it at the beginning of the pipeline to profile all middleware. But"}],"source_content_type":"application/octet-stream","patch_set":28,"id":"3ac371cc_abc43b40","line":199,"in_reply_to":"9ad45d7e_b285195b","updated":"2016-08-15 10:55:59.000000000","message":"The maximum number of containers to sync simultaneously (default\u003d1)","commit_id":"beffc5014c01ee62cecefe447122912129418155"}],"swift/container/replicator.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0f65be2c07a6c64d39e90e5c5454dcb44f3c7ddf","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 \u0027X-Container-Sync-To\u0027 not 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":3,"id":"da20952f_532ae930","line":210,"updated":"2015-09-08 18:36:55.000000000","message":"good cleanup","commit_id":"ee486b4ac60fe8e5f54373031856f8f71330d2be"}],"swift/container/sync.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0f65be2c07a6c64d39e90e5c5454dcb44f3c7ddf","unresolved":false,"context_lines":[{"line_number":329,"context_line":"    def _read_stats(self, fd):"},{"line_number":330,"context_line":"        try:"},{"line_number":331,"context_line":"            f \u003d os.fdopen(fd, \u0027r\u0027)"},{"line_number":332,"context_line":"            syncs, deletes, puts, skips, failures \u003d f.read().split()"},{"line_number":333,"context_line":"            self.container_syncs +\u003d int(syncs)"},{"line_number":334,"context_line":"            self.container_deletes +\u003d int(deletes)"},{"line_number":335,"context_line":"            self.container_puts +\u003d int(puts)"}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_339c155d","line":332,"updated":"2015-09-08 18:36:55.000000000","message":"seems like this would have a non-deterministic length\n\nWhile it\u0027s probably unlikely there would be a half-line on the wire - having two lines seems pretty likely - maybe could be addressed with a read*line*?\n\nWe\u0027ve been burnt to many times by fixed length splits for backward compatibility.  At a minimum we should split()[num_expected_args:] - but it would probably be better to make it a proper wire protocol like fixed-width-integer length followed by serialized key-value data.\n\nThat or just use the filesystem to collect stats...","commit_id":"ee486b4ac60fe8e5f54373031856f8f71330d2be"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"d13015d18bc1748d5ce16b0b8e9c4347331835cc","unresolved":false,"context_lines":[{"line_number":329,"context_line":"    def _read_stats(self, fd):"},{"line_number":330,"context_line":"        try:"},{"line_number":331,"context_line":"            f \u003d os.fdopen(fd, \u0027r\u0027)"},{"line_number":332,"context_line":"            syncs, deletes, puts, skips, failures \u003d f.read().split()"},{"line_number":333,"context_line":"            self.container_syncs +\u003d int(syncs)"},{"line_number":334,"context_line":"            self.container_deletes +\u003d int(deletes)"},{"line_number":335,"context_line":"            self.container_puts +\u003d int(puts)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba15a1d1_cda3c94e","line":332,"in_reply_to":"da20952f_339c155d","updated":"2015-09-12 21:29:53.000000000","message":"Right. changed as proposed to length + serialized dictionary\nWhile at it, also changed ContainerSync to hold the stats in a dictionary\nwhich IMO will make it easier to add more stats.","commit_id":"ee486b4ac60fe8e5f54373031856f8f71330d2be"},{"author":{"_account_id":5600,"name":"Brian Cline","email":"bcline@softlayer.com","username":"briancline"},"change_message_id":"327489f22072a6b1cdd20fb838ad63ce59ca659f","unresolved":false,"context_lines":[{"line_number":13,"context_line":"# See the License for the specific language governing permissions and"},{"line_number":14,"context_line":"# limitations under the License."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"from __future__ import print_function"},{"line_number":17,"context_line":"import errno"},{"line_number":18,"context_line":"import os"},{"line_number":19,"context_line":"import signal"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa80f949_34995c6f","line":16,"updated":"2015-11-03 23:10:03.000000000","message":"I couldn\u0027t find a print() in here anywhere -- did this get left in from debugging?","commit_id":"ce397a8501b92bc6393428df66b1ac78f7b346c2"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"62b70c66143bd341be3f4c19f0e51bb4d2805aaf","unresolved":false,"context_lines":[{"line_number":321,"context_line":"        }"},{"line_number":322,"context_line":"        try:"},{"line_number":323,"context_line":"            stats_string \u003d json.dumps(stats_dict)"},{"line_number":324,"context_line":"        except Exception:"},{"line_number":325,"context_line":"            self.logger.exception(_(\u0027Error serializing stats\u0027))"},{"line_number":326,"context_line":"            raise"},{"line_number":327,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"fa0719c6_47c1d82d","line":324,"range":{"start_line":324,"start_character":8,"end_line":324,"end_character":24},"updated":"2016-03-22 15:58:37.000000000","message":"Can we use more specific exception? (ValueError?)","commit_id":"a735accf2422b325bc7d210278fa63810332b5bf"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"62b70c66143bd341be3f4c19f0e51bb4d2805aaf","unresolved":false,"context_lines":[{"line_number":327,"context_line":""},{"line_number":328,"context_line":"        stats_string_len \u003d len(stats_string)"},{"line_number":329,"context_line":"        try:"},{"line_number":330,"context_line":"            f \u003d os.fdopen(fd, \u0027w\u0027)"},{"line_number":331,"context_line":"            f.write(\u0027%s\\n\u0027 % str(stats_string_len).zfill(self.stats_ipc_len))"},{"line_number":332,"context_line":"            f.write(stats_string)"},{"line_number":333,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa0719c6_c4b846e8","line":330,"range":{"start_line":330,"start_character":12,"end_line":330,"end_character":34},"updated":"2016-03-22 15:58:37.000000000","message":"How about using the with-statement here, like l340?","commit_id":"a735accf2422b325bc7d210278fa63810332b5bf"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"62b70c66143bd341be3f4c19f0e51bb4d2805aaf","unresolved":false,"context_lines":[{"line_number":330,"context_line":"            f \u003d os.fdopen(fd, \u0027w\u0027)"},{"line_number":331,"context_line":"            f.write(\u0027%s\\n\u0027 % str(stats_string_len).zfill(self.stats_ipc_len))"},{"line_number":332,"context_line":"            f.write(stats_string)"},{"line_number":333,"context_line":"        except Exception:"},{"line_number":334,"context_line":"            self.logger.exception(_(\u0027Error writing stats to pipe\u0027))"},{"line_number":335,"context_line":"        finally:"},{"line_number":336,"context_line":"            f.close()"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa0719c6_47aa7871","line":333,"range":{"start_line":333,"start_character":15,"end_line":333,"end_character":24},"updated":"2016-03-22 15:58:37.000000000","message":"Can we use more specific exception? (OSError?)","commit_id":"a735accf2422b325bc7d210278fa63810332b5bf"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"62b70c66143bd341be3f4c19f0e51bb4d2805aaf","unresolved":false,"context_lines":[{"line_number":330,"context_line":"            f \u003d os.fdopen(fd, \u0027w\u0027)"},{"line_number":331,"context_line":"            f.write(\u0027%s\\n\u0027 % str(stats_string_len).zfill(self.stats_ipc_len))"},{"line_number":332,"context_line":"            f.write(stats_string)"},{"line_number":333,"context_line":"        except Exception:"},{"line_number":334,"context_line":"            self.logger.exception(_(\u0027Error writing stats to pipe\u0027))"},{"line_number":335,"context_line":"        finally:"},{"line_number":336,"context_line":"            f.close()"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa0719c6_e410ea03","line":333,"range":{"start_line":333,"start_character":8,"end_line":333,"end_character":25},"updated":"2016-03-22 15:58:37.000000000","message":"Don\u0027t we need to reraise this exception?\n\nOr, Shouldn\u0027t we reraise any exceptions in _write_stats? (Raising exceptions seems to cause an unexpected death of subprocess.)","commit_id":"a735accf2422b325bc7d210278fa63810332b5bf"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"62b70c66143bd341be3f4c19f0e51bb4d2805aaf","unresolved":false,"context_lines":[{"line_number":341,"context_line":"                length_string \u003d f.read(self.stats_ipc_len + 1)"},{"line_number":342,"context_line":"                length \u003d int(length_string[:-1])"},{"line_number":343,"context_line":"                stats_string \u003d f.read(length)"},{"line_number":344,"context_line":"        except Exception:"},{"line_number":345,"context_line":"            self.logger.exception(_(\u0027Error reading stats from pipe\u0027))"},{"line_number":346,"context_line":"            return"},{"line_number":347,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"fa0719c6_c7eb28af","line":344,"range":{"start_line":344,"start_character":15,"end_line":344,"end_character":24},"updated":"2016-03-22 15:58:37.000000000","message":"ditto","commit_id":"a735accf2422b325bc7d210278fa63810332b5bf"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"62b70c66143bd341be3f4c19f0e51bb4d2805aaf","unresolved":false,"context_lines":[{"line_number":347,"context_line":""},{"line_number":348,"context_line":"        try:"},{"line_number":349,"context_line":"            stats \u003d json.loads(stats_string)"},{"line_number":350,"context_line":"        except Exception:"},{"line_number":351,"context_line":"            self.logger.exception("},{"line_number":352,"context_line":"                _(\u0027Error de-derializing stats %s\u0027), stats_string)"},{"line_number":353,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa0719c6_27d30406","line":350,"range":{"start_line":350,"start_character":15,"end_line":350,"end_character":24},"updated":"2016-03-22 15:58:37.000000000","message":"ditto","commit_id":"a735accf2422b325bc7d210278fa63810332b5bf"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"90a0bc37b28202105fa42fd350f417e58acff1c4","unresolved":false,"context_lines":[{"line_number":311,"context_line":"        }"},{"line_number":312,"context_line":"        try:"},{"line_number":313,"context_line":"            stats_string \u003d json.dumps(stats_dict)"},{"line_number":314,"context_line":"        except Exception:"},{"line_number":315,"context_line":"            self.logger.exception(_(\u0027Error serializing stats\u0027))"},{"line_number":316,"context_line":"            return"},{"line_number":317,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"fa0719c6_06b4a27d","line":314,"range":{"start_line":314,"start_character":15,"end_line":314,"end_character":24},"updated":"2016-03-24 04:42:10.000000000","message":"json.dumps seems to raise only TypeError, in this case.\n\nhttps://docs.python.org/2/library/json.html#json.dump\n\n# json.dumps seems to work similarly to json.dump","commit_id":"77d730237f6a1efa550a4dcb510763bff898e92f"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"90a0bc37b28202105fa42fd350f417e58acff1c4","unresolved":false,"context_lines":[{"line_number":321,"context_line":"                f.write(\u0027%s\\n\u0027 % str("},{"line_number":322,"context_line":"                    stats_string_len).zfill(self.stats_ipc_len))"},{"line_number":323,"context_line":"                f.write(stats_string)"},{"line_number":324,"context_line":"        except Exception:"},{"line_number":325,"context_line":"            self.logger.exception(_(\u0027Error writing stats to pipe\u0027))"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"    def _read_stats(self, fd):"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa0719c6_e6752ecb","line":324,"range":{"start_line":324,"start_character":15,"end_line":324,"end_character":24},"updated":"2016-03-24 04:42:10.000000000","message":"IOError","commit_id":"77d730237f6a1efa550a4dcb510763bff898e92f"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"90a0bc37b28202105fa42fd350f417e58acff1c4","unresolved":false,"context_lines":[{"line_number":330,"context_line":"                length_string \u003d f.read(self.stats_ipc_len + 1)"},{"line_number":331,"context_line":"                length \u003d int(length_string[:-1])"},{"line_number":332,"context_line":"                stats_string \u003d f.read(length)"},{"line_number":333,"context_line":"        except Exception:"},{"line_number":334,"context_line":"            self.logger.exception(_(\u0027Error reading stats from pipe\u0027))"},{"line_number":335,"context_line":"            return"},{"line_number":336,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"fa0719c6_864d529c","line":333,"range":{"start_line":333,"start_character":15,"end_line":333,"end_character":24},"updated":"2016-03-24 04:42:10.000000000","message":"IOError","commit_id":"77d730237f6a1efa550a4dcb510763bff898e92f"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"90a0bc37b28202105fa42fd350f417e58acff1c4","unresolved":false,"context_lines":[{"line_number":336,"context_line":""},{"line_number":337,"context_line":"        try:"},{"line_number":338,"context_line":"            stats \u003d json.loads(stats_string)"},{"line_number":339,"context_line":"        except Exception:"},{"line_number":340,"context_line":"            self.logger.exception("},{"line_number":341,"context_line":"                _(\u0027Error de-derializing stats %s\u0027), stats_string)"},{"line_number":342,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa0719c6_0629429e","line":339,"range":{"start_line":339,"start_character":15,"end_line":339,"end_character":24},"updated":"2016-03-24 04:42:10.000000000","message":"There are no descriptions about exec type in load/loads, but it seems that loads only raises ValueError, which is raised by JSONDecoder[1].\n\n[1] https://docs.python.org/2/library/json.html#json.JSONDecoder","commit_id":"77d730237f6a1efa550a4dcb510763bff898e92f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e4a7e350a9c415f16824e51020420de29ffec3f0","unresolved":false,"context_lines":[{"line_number":209,"context_line":"        #:     but are not yet cleared from the sync store."},{"line_number":210,"context_line":"        #: failures: The number of containers that had a failure of some type."},{"line_number":211,"context_line":"        self.stats \u003d collections.defaultdict(int)"},{"line_number":212,"context_line":"        self.stats.clear()"},{"line_number":213,"context_line":"        self.stats_ipc_len \u003d 15"},{"line_number":214,"context_line":"        #: Time of last stats report."},{"line_number":215,"context_line":"        self.reported \u003d time()"}],"source_content_type":"text/x-python","patch_set":21,"id":"dada55a8_1e76b4d6","line":212,"updated":"2016-07-21 12:20:36.000000000","message":"nit: not necessary to clear a new defaultdict","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"4f74852fad09fa761891a9372395c6639121ec9e","unresolved":false,"context_lines":[{"line_number":209,"context_line":"        #:     but are not yet cleared from the sync store."},{"line_number":210,"context_line":"        #: failures: The number of containers that had a failure of some type."},{"line_number":211,"context_line":"        self.stats \u003d collections.defaultdict(int)"},{"line_number":212,"context_line":"        self.stats.clear()"},{"line_number":213,"context_line":"        self.stats_ipc_len \u003d 15"},{"line_number":214,"context_line":"        #: Time of last stats report."},{"line_number":215,"context_line":"        self.reported \u003d time()"}],"source_content_type":"text/x-python","patch_set":21,"id":"dada55a8_f8792ac9","line":212,"in_reply_to":"dada55a8_1e76b4d6","updated":"2016-08-01 13:47:20.000000000","message":"Done","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e4a7e350a9c415f16824e51020420de29ffec3f0","unresolved":false,"context_lines":[{"line_number":217,"context_line":"        #: swift.common.ring.Ring for locating containers."},{"line_number":218,"context_line":"        self.container_ring \u003d container_ring or Ring(self.swift_dir,"},{"line_number":219,"context_line":"                                                     ring_name\u003d\u0027container\u0027)"},{"line_number":220,"context_line":"        #: The maxumum number of sync processes running in parallel"},{"line_number":221,"context_line":"        self.processes \u003d int(conf.get(\u0027processes\u0027, 1))"},{"line_number":222,"context_line":"        bind_ip \u003d conf.get(\u0027bind_ip\u0027, \u00270.0.0.0\u0027)"},{"line_number":223,"context_line":"        self._myips \u003d whataremyips(bind_ip)"}],"source_content_type":"text/x-python","patch_set":21,"id":"dada55a8_de3d3cde","line":220,"updated":"2016-07-21 12:20:36.000000000","message":"nit: s/maxumum/maximum/","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"4f74852fad09fa761891a9372395c6639121ec9e","unresolved":false,"context_lines":[{"line_number":217,"context_line":"        #: swift.common.ring.Ring for locating containers."},{"line_number":218,"context_line":"        self.container_ring \u003d container_ring or Ring(self.swift_dir,"},{"line_number":219,"context_line":"                                                     ring_name\u003d\u0027container\u0027)"},{"line_number":220,"context_line":"        #: The maxumum number of sync processes running in parallel"},{"line_number":221,"context_line":"        self.processes \u003d int(conf.get(\u0027processes\u0027, 1))"},{"line_number":222,"context_line":"        bind_ip \u003d conf.get(\u0027bind_ip\u0027, \u00270.0.0.0\u0027)"},{"line_number":223,"context_line":"        self._myips \u003d whataremyips(bind_ip)"}],"source_content_type":"text/x-python","patch_set":21,"id":"dada55a8_986e2efe","line":220,"in_reply_to":"dada55a8_de3d3cde","updated":"2016-08-01 13:47:20.000000000","message":"Done","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e4a7e350a9c415f16824e51020420de29ffec3f0","unresolved":false,"context_lines":[{"line_number":266,"context_line":"                    self._wait_for_child()"},{"line_number":267,"context_line":""},{"line_number":268,"context_line":"                read_fd, write_fd \u003d os.pipe()"},{"line_number":269,"context_line":"                pid \u003d os.fork()"},{"line_number":270,"context_line":"                if pid:"},{"line_number":271,"context_line":"                    os.close(write_fd)"},{"line_number":272,"context_line":"                    self.pid2file[pid] \u003d read_fd"}],"source_content_type":"text/x-python","patch_set":21,"id":"dada55a8_99a11efb","line":269,"updated":"2016-07-21 12:20:36.000000000","message":"so we are forking even if self.processes \u003d\u003d 1 which is the default. That means that by default upgrade to this version results in an extra forked process. Maybe we can only fork if processes \u003e 1?","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"4f74852fad09fa761891a9372395c6639121ec9e","unresolved":false,"context_lines":[{"line_number":266,"context_line":"                    self._wait_for_child()"},{"line_number":267,"context_line":""},{"line_number":268,"context_line":"                read_fd, write_fd \u003d os.pipe()"},{"line_number":269,"context_line":"                pid \u003d os.fork()"},{"line_number":270,"context_line":"                if pid:"},{"line_number":271,"context_line":"                    os.close(write_fd)"},{"line_number":272,"context_line":"                    self.pid2file[pid] \u003d read_fd"}],"source_content_type":"text/x-python","patch_set":21,"id":"bacf61ea_aa3488fb","line":269,"in_reply_to":"dada55a8_99a11efb","updated":"2016-08-01 13:47:20.000000000","message":"True, one process runs container sync, the other gather the stats.\n\nDo you see an elegant way of refactoring the code without copying the lines twice? that would be less readable..","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e4a7e350a9c415f16824e51020420de29ffec3f0","unresolved":false,"context_lines":[{"line_number":276,"context_line":"                    self.container_stats.clear()"},{"line_number":277,"context_line":"                    self.container_sync(path)"},{"line_number":278,"context_line":"                    self._write_stats(write_fd)"},{"line_number":279,"context_line":"                    sys.exit()"},{"line_number":280,"context_line":""},{"line_number":281,"context_line":"                if time() - self.reported \u003e\u003d 3600:  # once an hour"},{"line_number":282,"context_line":"                    self.report()"}],"source_content_type":"text/x-python","patch_set":21,"id":"dada55a8_d229ace7","line":279,"updated":"2016-07-21 12:20:36.000000000","message":"to be sure that sys.exit() is always called in the forked child process, add a try/finally, i.e.:\n\n                try:\n                    signal.signal(signal.SIGTERM, signal.SIG_DFL)\n                    os.close(read_fd)\n                    self.container_stats.clear()\n                    self.container_sync(path)\n                    self._write_stats(write_fd)\n                finally:\n                    sys.exit()\n\nRight now I do not think there is a problem: any exception would be raised and cause run_forever to terminate, so this try/finally is not strictly necessary. But if ever an exception handler were added in the run_forever while loop then there is a danger that the forked process raises an exception, never reaches sys.exit and forks another process.\n\nSee https://bugs.launchpad.net/swift/+bug/1375348 as an example of what might happen.","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"4f74852fad09fa761891a9372395c6639121ec9e","unresolved":false,"context_lines":[{"line_number":276,"context_line":"                    self.container_stats.clear()"},{"line_number":277,"context_line":"                    self.container_sync(path)"},{"line_number":278,"context_line":"                    self._write_stats(write_fd)"},{"line_number":279,"context_line":"                    sys.exit()"},{"line_number":280,"context_line":""},{"line_number":281,"context_line":"                if time() - self.reported \u003e\u003d 3600:  # once an hour"},{"line_number":282,"context_line":"                    self.report()"}],"source_content_type":"text/x-python","patch_set":21,"id":"dada55a8_b897b238","line":279,"in_reply_to":"dada55a8_d229ace7","updated":"2016-08-01 13:47:20.000000000","message":"Done","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e4a7e350a9c415f16824e51020420de29ffec3f0","unresolved":false,"context_lines":[{"line_number":311,"context_line":"            \u0027puts\u0027: self.stats[\u0027puts\u0027],"},{"line_number":312,"context_line":"            \u0027skips\u0027: self.stats[\u0027skips\u0027],"},{"line_number":313,"context_line":"            \u0027failures\u0027: self.stats[\u0027failures\u0027]"},{"line_number":314,"context_line":"        }"},{"line_number":315,"context_line":"        try:"},{"line_number":316,"context_line":"            stats_string \u003d json.dumps(stats_dict)"},{"line_number":317,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":21,"id":"dada55a8_524dbc98","line":314,"updated":"2016-07-21 12:20:36.000000000","message":"why construct another dict, just json.dumps(self.stats)?","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"4f74852fad09fa761891a9372395c6639121ec9e","unresolved":false,"context_lines":[{"line_number":311,"context_line":"            \u0027puts\u0027: self.stats[\u0027puts\u0027],"},{"line_number":312,"context_line":"            \u0027skips\u0027: self.stats[\u0027skips\u0027],"},{"line_number":313,"context_line":"            \u0027failures\u0027: self.stats[\u0027failures\u0027]"},{"line_number":314,"context_line":"        }"},{"line_number":315,"context_line":"        try:"},{"line_number":316,"context_line":"            stats_string \u003d json.dumps(stats_dict)"},{"line_number":317,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":21,"id":"dada55a8_b8ae927e","line":314,"in_reply_to":"dada55a8_524dbc98","updated":"2016-08-01 13:47:20.000000000","message":"Done","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e4a7e350a9c415f16824e51020420de29ffec3f0","unresolved":false,"context_lines":[{"line_number":314,"context_line":"        }"},{"line_number":315,"context_line":"        try:"},{"line_number":316,"context_line":"            stats_string \u003d json.dumps(stats_dict)"},{"line_number":317,"context_line":"        except Exception:"},{"line_number":318,"context_line":"            self.logger.exception(_(\u0027Error serializing stats\u0027))"},{"line_number":319,"context_line":"            return"},{"line_number":320,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"dada55a8_d261ac06","line":317,"updated":"2016-07-21 12:20:36.000000000","message":"use a more specific exception","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"4f74852fad09fa761891a9372395c6639121ec9e","unresolved":false,"context_lines":[{"line_number":314,"context_line":"        }"},{"line_number":315,"context_line":"        try:"},{"line_number":316,"context_line":"            stats_string \u003d json.dumps(stats_dict)"},{"line_number":317,"context_line":"        except Exception:"},{"line_number":318,"context_line":"            self.logger.exception(_(\u0027Error serializing stats\u0027))"},{"line_number":319,"context_line":"            return"},{"line_number":320,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"dada55a8_38a90225","line":317,"in_reply_to":"dada55a8_d261ac06","updated":"2016-08-01 13:47:20.000000000","message":"Done","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e4a7e350a9c415f16824e51020420de29ffec3f0","unresolved":false,"context_lines":[{"line_number":332,"context_line":"            with os.fdopen(fd, \u0027r\u0027) as f:"},{"line_number":333,"context_line":"                length_string \u003d f.read(self.stats_ipc_len + 1)"},{"line_number":334,"context_line":"                length \u003d int(length_string[:-1])"},{"line_number":335,"context_line":"                stats_string \u003d f.read(length)"},{"line_number":336,"context_line":"        except Exception:"},{"line_number":337,"context_line":"            self.logger.exception(_(\u0027Error reading stats from pipe\u0027))"},{"line_number":338,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":21,"id":"dada55a8_556126a8","line":335,"updated":"2016-07-21 12:20:36.000000000","message":"could readline be used? then length would not need to be sent and parsed.","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e4a7e350a9c415f16824e51020420de29ffec3f0","unresolved":false,"context_lines":[{"line_number":339,"context_line":""},{"line_number":340,"context_line":"        try:"},{"line_number":341,"context_line":"            stats \u003d json.loads(stats_string)"},{"line_number":342,"context_line":"        except Exception:"},{"line_number":343,"context_line":"            self.logger.exception("},{"line_number":344,"context_line":"                _(\u0027Error de-derializing stats %s\u0027), stats_string)"},{"line_number":345,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":21,"id":"dada55a8_15032eb9","line":342,"updated":"2016-07-21 12:20:36.000000000","message":"as above, use more specific exception","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"4f74852fad09fa761891a9372395c6639121ec9e","unresolved":false,"context_lines":[{"line_number":339,"context_line":""},{"line_number":340,"context_line":"        try:"},{"line_number":341,"context_line":"            stats \u003d json.loads(stats_string)"},{"line_number":342,"context_line":"        except Exception:"},{"line_number":343,"context_line":"            self.logger.exception("},{"line_number":344,"context_line":"                _(\u0027Error de-derializing stats %s\u0027), stats_string)"},{"line_number":345,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":21,"id":"dada55a8_18a4fe48","line":342,"in_reply_to":"dada55a8_15032eb9","updated":"2016-08-01 13:47:20.000000000","message":"Done","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e4a7e350a9c415f16824e51020420de29ffec3f0","unresolved":false,"context_lines":[{"line_number":341,"context_line":"            stats \u003d json.loads(stats_string)"},{"line_number":342,"context_line":"        except Exception:"},{"line_number":343,"context_line":"            self.logger.exception("},{"line_number":344,"context_line":"                _(\u0027Error de-derializing stats %s\u0027), stats_string)"},{"line_number":345,"context_line":"            return"},{"line_number":346,"context_line":""},{"line_number":347,"context_line":"        self.stats[\u0027syncs\u0027] +\u003d int(stats[\u0027syncs\u0027])"}],"source_content_type":"text/x-python","patch_set":21,"id":"dada55a8_151a0e17","line":344,"updated":"2016-07-21 12:20:36.000000000","message":"typo: de-serializing","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"4f74852fad09fa761891a9372395c6639121ec9e","unresolved":false,"context_lines":[{"line_number":341,"context_line":"            stats \u003d json.loads(stats_string)"},{"line_number":342,"context_line":"        except Exception:"},{"line_number":343,"context_line":"            self.logger.exception("},{"line_number":344,"context_line":"                _(\u0027Error de-derializing stats %s\u0027), stats_string)"},{"line_number":345,"context_line":"            return"},{"line_number":346,"context_line":""},{"line_number":347,"context_line":"        self.stats[\u0027syncs\u0027] +\u003d int(stats[\u0027syncs\u0027])"}],"source_content_type":"text/x-python","patch_set":21,"id":"dada55a8_2431dea2","line":344,"in_reply_to":"dada55a8_151a0e17","updated":"2016-08-01 13:47:20.000000000","message":"Done","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e4a7e350a9c415f16824e51020420de29ffec3f0","unresolved":false,"context_lines":[{"line_number":348,"context_line":"        self.stats[\u0027deletes\u0027] +\u003d int(stats[\u0027deletes\u0027])"},{"line_number":349,"context_line":"        self.stats[\u0027puts\u0027] +\u003d int(stats[\u0027puts\u0027])"},{"line_number":350,"context_line":"        self.stats[\u0027skips\u0027] +\u003d int(stats[\u0027skips\u0027])"},{"line_number":351,"context_line":"        self.stats[\u0027failures\u0027] +\u003d int(stats[\u0027failures\u0027])"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":"    def report(self):"},{"line_number":354,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":21,"id":"dada55a8_15c84e73","line":351,"updated":"2016-07-21 12:20:36.000000000","message":"or:\n\n  for k,v in stats.items():\n      self.stats[k] +\u003d v","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"4f74852fad09fa761891a9372395c6639121ec9e","unresolved":false,"context_lines":[{"line_number":348,"context_line":"        self.stats[\u0027deletes\u0027] +\u003d int(stats[\u0027deletes\u0027])"},{"line_number":349,"context_line":"        self.stats[\u0027puts\u0027] +\u003d int(stats[\u0027puts\u0027])"},{"line_number":350,"context_line":"        self.stats[\u0027skips\u0027] +\u003d int(stats[\u0027skips\u0027])"},{"line_number":351,"context_line":"        self.stats[\u0027failures\u0027] +\u003d int(stats[\u0027failures\u0027])"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":"    def report(self):"},{"line_number":354,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":21,"id":"dada55a8_0402dab3","line":351,"in_reply_to":"dada55a8_15c84e73","updated":"2016-08-01 13:47:20.000000000","message":"Done","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e4a7e350a9c415f16824e51020420de29ffec3f0","unresolved":false,"context_lines":[{"line_number":363,"context_line":"             \u0027delete\u0027: self.stats[\u0027deletes\u0027],"},{"line_number":364,"context_line":"             \u0027put\u0027: self.stats[\u0027puts\u0027],"},{"line_number":365,"context_line":"             \u0027skip\u0027: self.stats[\u0027skips\u0027],"},{"line_number":366,"context_line":"             \u0027fail\u0027: self.stats[\u0027failures\u0027]})"},{"line_number":367,"context_line":"        self.stats.clear()"},{"line_number":368,"context_line":"        self.reported \u003d time()"},{"line_number":369,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"dada55a8_5592c64c","line":366,"updated":"2016-07-21 12:20:36.000000000","message":"or:\n\n  report_dict \u003d dict(self.stats)\n  report_dict[\u0027time\u0027] \u003d ctime(self.reported)        \n  self.logger.info(\n            _(\u0027Since %(time)s: %(sync)s synced [%(delete)s deletes, %(put)s \u0027\n              \u0027puts], %(skip)s skipped, %(fail)s failed\u0027), report_dict)","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"4f74852fad09fa761891a9372395c6639121ec9e","unresolved":false,"context_lines":[{"line_number":363,"context_line":"             \u0027delete\u0027: self.stats[\u0027deletes\u0027],"},{"line_number":364,"context_line":"             \u0027put\u0027: self.stats[\u0027puts\u0027],"},{"line_number":365,"context_line":"             \u0027skip\u0027: self.stats[\u0027skips\u0027],"},{"line_number":366,"context_line":"             \u0027fail\u0027: self.stats[\u0027failures\u0027]})"},{"line_number":367,"context_line":"        self.stats.clear()"},{"line_number":368,"context_line":"        self.reported \u003d time()"},{"line_number":369,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"dada55a8_64275663","line":366,"in_reply_to":"dada55a8_5592c64c","updated":"2016-08-01 13:47:20.000000000","message":"Problem is in case any of the stats fields \u003d\u003d 0, if converting to dict the print may fail.\nexample: self.stats \u003d {\u0027puts: 11\u0027} then\n\n        report_dict \u003d dict(self.stats)\n        report_dict[\u0027time\u0027] \u003d ctime(self.reported)        \n        self.logger.info(\n            _(\u0027Since time: %(time)s, synced: %(sync)s, [deletes: %(delete)s, \u0027\n              \u0027puts: %(put)s], skipped: %(skip)s, failed: %(fail)s\u0027),\n            report_dict) \n\nwould faile on sync, delete and skipped fields since they are not in the dict","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"94218bc21abbb0d90c509e290ac21d9a97f96dea","unresolved":false,"context_lines":[{"line_number":273,"context_line":"                        signal.signal(signal.SIGTERM, signal.SIG_DFL)"},{"line_number":274,"context_line":"                        os.close(read_fd)"},{"line_number":275,"context_line":"                        self.container_stats.clear()"},{"line_number":276,"context_line":"                        self.stats.clear()"},{"line_number":277,"context_line":"                        self.container_sync(path)"},{"line_number":278,"context_line":"                        self._write_stats(write_fd)"},{"line_number":279,"context_line":"                    finally:"}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_6230bae9","line":276,"updated":"2016-08-03 18:10:06.000000000","message":"The use of self to hold stats and container_stats is quite hard to follow - I wonder if we could avoid using the instance variable in the child process?\n\n... see my patch for this.","commit_id":"b06a315c3ab3b2cdf96e4feacdcfaef21feabea5"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"f404b9371598a8fdc9e2797d6eadd23949c26df4","unresolved":false,"context_lines":[{"line_number":273,"context_line":"                        signal.signal(signal.SIGTERM, signal.SIG_DFL)"},{"line_number":274,"context_line":"                        os.close(read_fd)"},{"line_number":275,"context_line":"                        self.container_stats.clear()"},{"line_number":276,"context_line":"                        self.stats.clear()"},{"line_number":277,"context_line":"                        self.container_sync(path)"},{"line_number":278,"context_line":"                        self._write_stats(write_fd)"},{"line_number":279,"context_line":"                    finally:"}],"source_content_type":"text/x-python","patch_set":24,"id":"9ad45d7e_75aa2b72","line":276,"in_reply_to":"bacf61ea_6230bae9","updated":"2016-08-08 14:36:37.000000000","message":"I like this idea. Any impact on future threads work (followup patch)?","commit_id":"b06a315c3ab3b2cdf96e4feacdcfaef21feabea5"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"94218bc21abbb0d90c509e290ac21d9a97f96dea","unresolved":false,"context_lines":[{"line_number":279,"context_line":"                    finally:"},{"line_number":280,"context_line":"                        sys.exit()"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"                if time() - self.reported \u003e\u003d 3600:  # once an hour"},{"line_number":283,"context_line":"                    self.report()"},{"line_number":284,"context_line":"            # Done looping over synced containers"},{"line_number":285,"context_line":"            # make sure all workers are done"}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_9a8992cd","line":282,"updated":"2016-08-03 18:10:06.000000000","message":"(this is same on master) - we get no log, ever, if there are no containers to sync? A log of no activity once per hour might be betters.","commit_id":"b06a315c3ab3b2cdf96e4feacdcfaef21feabea5"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"f404b9371598a8fdc9e2797d6eadd23949c26df4","unresolved":false,"context_lines":[{"line_number":279,"context_line":"                    finally:"},{"line_number":280,"context_line":"                        sys.exit()"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"                if time() - self.reported \u003e\u003d 3600:  # once an hour"},{"line_number":283,"context_line":"                    self.report()"},{"line_number":284,"context_line":"            # Done looping over synced containers"},{"line_number":285,"context_line":"            # make sure all workers are done"}],"source_content_type":"text/x-python","patch_set":24,"id":"9ad45d7e_35f55383","line":282,"in_reply_to":"bacf61ea_9a8992cd","updated":"2016-08-08 14:36:37.000000000","message":"Good catch, will move the report to wait_for_child and add another report at the end - so report would be made regardless of activity too","commit_id":"b06a315c3ab3b2cdf96e4feacdcfaef21feabea5"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"94218bc21abbb0d90c509e290ac21d9a97f96dea","unresolved":false,"context_lines":[{"line_number":280,"context_line":"                        sys.exit()"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"                if time() - self.reported \u003e\u003d 3600:  # once an hour"},{"line_number":283,"context_line":"                    self.report()"},{"line_number":284,"context_line":"            # Done looping over synced containers"},{"line_number":285,"context_line":"            # make sure all workers are done"},{"line_number":286,"context_line":"            while self.pid2file:"}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_0d4d6e72","line":283,"updated":"2016-08-03 18:10:06.000000000","message":"I think this might be better done in _wait_for_child. That addresses my comment at line 287","commit_id":"b06a315c3ab3b2cdf96e4feacdcfaef21feabea5"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"f404b9371598a8fdc9e2797d6eadd23949c26df4","unresolved":false,"context_lines":[{"line_number":280,"context_line":"                        sys.exit()"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"                if time() - self.reported \u003e\u003d 3600:  # once an hour"},{"line_number":283,"context_line":"                    self.report()"},{"line_number":284,"context_line":"            # Done looping over synced containers"},{"line_number":285,"context_line":"            # make sure all workers are done"},{"line_number":286,"context_line":"            while self.pid2file:"}],"source_content_type":"text/x-python","patch_set":24,"id":"9ad45d7e_75efcbb0","line":283,"in_reply_to":"bacf61ea_0d4d6e72","updated":"2016-08-08 14:36:37.000000000","message":"Done","commit_id":"b06a315c3ab3b2cdf96e4feacdcfaef21feabea5"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"94218bc21abbb0d90c509e290ac21d9a97f96dea","unresolved":false,"context_lines":[{"line_number":282,"context_line":"                if time() - self.reported \u003e\u003d 3600:  # once an hour"},{"line_number":283,"context_line":"                    self.report()"},{"line_number":284,"context_line":"            # Done looping over synced containers"},{"line_number":285,"context_line":"            # make sure all workers are done"},{"line_number":286,"context_line":"            while self.pid2file:"},{"line_number":287,"context_line":"                self._wait_for_child()"},{"line_number":288,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_7af3fe9f","line":285,"updated":"2016-08-03 18:10:06.000000000","message":"I spent some time wondering why this was necessary - why not start iterating over synced_containers_generator again? Then realised that is the the number of sync containers is less than number of workers then we could end up with more than worker on the same container :/\n\nA note in the comment might be useful.","commit_id":"b06a315c3ab3b2cdf96e4feacdcfaef21feabea5"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"f404b9371598a8fdc9e2797d6eadd23949c26df4","unresolved":false,"context_lines":[{"line_number":282,"context_line":"                if time() - self.reported \u003e\u003d 3600:  # once an hour"},{"line_number":283,"context_line":"                    self.report()"},{"line_number":284,"context_line":"            # Done looping over synced containers"},{"line_number":285,"context_line":"            # make sure all workers are done"},{"line_number":286,"context_line":"            while self.pid2file:"},{"line_number":287,"context_line":"                self._wait_for_child()"},{"line_number":288,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"9ad45d7e_b52f83f5","line":285,"in_reply_to":"bacf61ea_7af3fe9f","updated":"2016-08-08 14:36:37.000000000","message":"Done","commit_id":"b06a315c3ab3b2cdf96e4feacdcfaef21feabea5"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"94218bc21abbb0d90c509e290ac21d9a97f96dea","unresolved":false,"context_lines":[{"line_number":284,"context_line":"            # Done looping over synced containers"},{"line_number":285,"context_line":"            # make sure all workers are done"},{"line_number":286,"context_line":"            while self.pid2file:"},{"line_number":287,"context_line":"                self._wait_for_child()"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"            elapsed \u003d time() - begin"},{"line_number":290,"context_line":"            if elapsed \u003c self.interval:"}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_dabe6afe","line":287,"updated":"2016-08-03 18:10:06.000000000","message":"I think there should be a self.report() after each worker completes, if an hour has passed. Otherwise the next report could be delayed by \u003e\u003e hour","commit_id":"b06a315c3ab3b2cdf96e4feacdcfaef21feabea5"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"f404b9371598a8fdc9e2797d6eadd23949c26df4","unresolved":false,"context_lines":[{"line_number":284,"context_line":"            # Done looping over synced containers"},{"line_number":285,"context_line":"            # make sure all workers are done"},{"line_number":286,"context_line":"            while self.pid2file:"},{"line_number":287,"context_line":"                self._wait_for_child()"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"            elapsed \u003d time() - begin"},{"line_number":290,"context_line":"            if elapsed \u003c self.interval:"}],"source_content_type":"text/x-python","patch_set":24,"id":"9ad45d7e_d57837fb","line":287,"in_reply_to":"bacf61ea_dabe6afe","updated":"2016-08-08 14:36:37.000000000","message":"V","commit_id":"b06a315c3ab3b2cdf96e4feacdcfaef21feabea5"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"94218bc21abbb0d90c509e290ac21d9a97f96dea","unresolved":false,"context_lines":[{"line_number":314,"context_line":""},{"line_number":315,"context_line":"        try:"},{"line_number":316,"context_line":"            with os.fdopen(fd, \u0027w\u0027) as f:"},{"line_number":317,"context_line":"                f.write(stats_string + \"\\n\")"},{"line_number":318,"context_line":"        except OSError:"},{"line_number":319,"context_line":"            self.logger.exception(_(\u0027Error writing stats to pipe\u0027))"},{"line_number":320,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_b60bdf7c","line":317,"updated":"2016-08-03 18:10:06.000000000","message":"this should be done with a timeout, otherwise if the parent fails to read the pipe this will block forever","commit_id":"b06a315c3ab3b2cdf96e4feacdcfaef21feabea5"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"f404b9371598a8fdc9e2797d6eadd23949c26df4","unresolved":false,"context_lines":[{"line_number":314,"context_line":""},{"line_number":315,"context_line":"        try:"},{"line_number":316,"context_line":"            with os.fdopen(fd, \u0027w\u0027) as f:"},{"line_number":317,"context_line":"                f.write(stats_string + \"\\n\")"},{"line_number":318,"context_line":"        except OSError:"},{"line_number":319,"context_line":"            self.logger.exception(_(\u0027Error writing stats to pipe\u0027))"},{"line_number":320,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"9ad45d7e_22d6b6c5","line":317,"in_reply_to":"bacf61ea_b60bdf7c","updated":"2016-08-08 14:36:37.000000000","message":"Looking at container updater and obj auditor using the same approach. Should patcher.monkey_patch(all\u003dFalse, socket\u003dTrue, select\u003dTrue, thread\u003dTrue) be added here?","commit_id":"b06a315c3ab3b2cdf96e4feacdcfaef21feabea5"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"94218bc21abbb0d90c509e290ac21d9a97f96dea","unresolved":false,"context_lines":[{"line_number":320,"context_line":""},{"line_number":321,"context_line":"    def _read_stats(self, fd):"},{"line_number":322,"context_line":"        try:"},{"line_number":323,"context_line":"            with os.fdopen(fd, \u0027r\u0027) as f:"},{"line_number":324,"context_line":"                stats_string \u003d f.readline()"},{"line_number":325,"context_line":"        except OSError:"},{"line_number":326,"context_line":"            self.logger.exception(_(\u0027Error reading stats from pipe\u0027))"}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_d622d300","line":323,"updated":"2016-08-03 18:10:06.000000000","message":"probably also needs to be done with a timeout","commit_id":"b06a315c3ab3b2cdf96e4feacdcfaef21feabea5"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"94218bc21abbb0d90c509e290ac21d9a97f96dea","unresolved":false,"context_lines":[{"line_number":499,"context_line":"                                          info, broker.get_max_row())"},{"line_number":500,"context_line":"        except (Exception, Timeout):"},{"line_number":501,"context_line":"            self.stats[\u0027failures\u0027] +\u003d 1"},{"line_number":502,"context_line":"            self.logger.increment(\u0027failures\u0027)"},{"line_number":503,"context_line":"            self.logger.exception(_(\u0027ERROR Syncing %s\u0027),"},{"line_number":504,"context_line":"                                  broker if broker else path)"},{"line_number":505,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_56440ab6","line":502,"updated":"2016-08-03 18:10:06.000000000","message":"which logger instance is being incremented in the child process?","commit_id":"b06a315c3ab3b2cdf96e4feacdcfaef21feabea5"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"f404b9371598a8fdc9e2797d6eadd23949c26df4","unresolved":false,"context_lines":[{"line_number":499,"context_line":"                                          info, broker.get_max_row())"},{"line_number":500,"context_line":"        except (Exception, Timeout):"},{"line_number":501,"context_line":"            self.stats[\u0027failures\u0027] +\u003d 1"},{"line_number":502,"context_line":"            self.logger.increment(\u0027failures\u0027)"},{"line_number":503,"context_line":"            self.logger.exception(_(\u0027ERROR Syncing %s\u0027),"},{"line_number":504,"context_line":"                                  broker if broker else path)"},{"line_number":505,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"9ad45d7e_22a4f655","line":502,"in_reply_to":"bacf61ea_56440ab6","updated":"2016-08-08 14:36:37.000000000","message":"Same logger (created with fork)","commit_id":"b06a315c3ab3b2cdf96e4feacdcfaef21feabea5"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"505e100dd79874025a6d6ffcf516fc6a4f3ecac1","unresolved":false,"context_lines":[{"line_number":244,"context_line":"            self._read_stats(self.pid2file[pid])"},{"line_number":245,"context_line":"        finally:"},{"line_number":246,"context_line":"            del self.pid2file[pid]"},{"line_number":247,"context_line":"            self.periodic_report()"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"    def _update_stats(self, container_stats):"},{"line_number":250,"context_line":"        for key in (\u0027puts\u0027, \u0027deletes\u0027, \u0027failures\u0027, \u0027skips\u0027, \u0027syncs\u0027):"}],"source_content_type":"text/x-python","patch_set":28,"id":"9ad45d7e_f43f86b1","line":247,"updated":"2016-08-12 17:18:07.000000000","message":"nice","commit_id":"beffc5014c01ee62cecefe447122912129418155"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"505e100dd79874025a6d6ffcf516fc6a4f3ecac1","unresolved":false,"context_lines":[{"line_number":256,"context_line":"        # Block until a worker process is available - can not"},{"line_number":257,"context_line":"        # fork more than # processes allocation defined"},{"line_number":258,"context_line":"        while len(self.pid2file) \u003e\u003d self.processes:"},{"line_number":259,"context_line":"            self._wait_for_child()"},{"line_number":260,"context_line":""},{"line_number":261,"context_line":"        read_fd, write_fd \u003d os.pipe()"},{"line_number":262,"context_line":"        pid \u003d os.fork()"}],"source_content_type":"text/x-python","patch_set":28,"id":"9ad45d7e_2ee26f2f","line":259,"updated":"2016-08-12 17:18:07.000000000","message":"I\u0027m a little nervous about moving this code into _do_fork, because we mock _do_fork in test_run_forever, and with this change that mocking means that the conditional _wait_for_child is not called after the job for each path is \"forked\". As a result, the pid2file dict can actually grow larger than the number of allowed processes during the tests.\n\nIllustrated by this diff:\nhttps://gist.github.com/alistairncoles/e35f00e59a1dbf2926681c531a60f7d8","commit_id":"beffc5014c01ee62cecefe447122912129418155"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"b2d0923c9b7e6b2d377c7da5ea5b7722dc242123","unresolved":false,"context_lines":[{"line_number":256,"context_line":"        # Block until a worker process is available - can not"},{"line_number":257,"context_line":"        # fork more than # processes allocation defined"},{"line_number":258,"context_line":"        while len(self.pid2file) \u003e\u003d self.processes:"},{"line_number":259,"context_line":"            self._wait_for_child()"},{"line_number":260,"context_line":""},{"line_number":261,"context_line":"        read_fd, write_fd \u003d os.pipe()"},{"line_number":262,"context_line":"        pid \u003d os.fork()"}],"source_content_type":"text/x-python","patch_set":28,"id":"3ac371cc_ebc63315","line":259,"in_reply_to":"9ad45d7e_2ee26f2f","updated":"2016-08-15 10:55:59.000000000","message":"True. Other option is to change the _do_fork mock in test_run_forever to call wait too","commit_id":"beffc5014c01ee62cecefe447122912129418155"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"505e100dd79874025a6d6ffcf516fc6a4f3ecac1","unresolved":false,"context_lines":[{"line_number":294,"context_line":"                self._wait_for_child()"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"            end \u003d time()"},{"line_number":297,"context_line":"            self.periodic_report(end)"},{"line_number":298,"context_line":"            elapsed \u003d end - begin"},{"line_number":299,"context_line":"            if elapsed \u003c self.interval:"},{"line_number":300,"context_line":"                sleep(self.interval - elapsed)"}],"source_content_type":"text/x-python","patch_set":28,"id":"9ad45d7e_94ec4a2b","line":297,"updated":"2016-08-12 17:18:07.000000000","message":"nit: I like periodic_report, nice refactoring. But if we don\u0027t care about the *exact* matching of time here, we could just use\n\n  self.periodic_report\n\nand simplify the signature","commit_id":"beffc5014c01ee62cecefe447122912129418155"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"505e100dd79874025a6d6ffcf516fc6a4f3ecac1","unresolved":false,"context_lines":[{"line_number":316,"context_line":"    def _write_stats(self, stats):"},{"line_number":317,"context_line":"        try:"},{"line_number":318,"context_line":"            stats_string \u003d json.dumps(stats)"},{"line_number":319,"context_line":"        except (ValueError, TypeError):"},{"line_number":320,"context_line":"            self.logger.exception(_(\u0027Error serializing stats\u0027))"},{"line_number":321,"context_line":"            return"},{"line_number":322,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"9ad45d7e_c1b38a4b","line":319,"updated":"2016-08-12 17:18:07.000000000","message":"I could not see a way to get a ValueError from json.dumps(stats)","commit_id":"beffc5014c01ee62cecefe447122912129418155"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"b2d0923c9b7e6b2d377c7da5ea5b7722dc242123","unresolved":false,"context_lines":[{"line_number":316,"context_line":"    def _write_stats(self, stats):"},{"line_number":317,"context_line":"        try:"},{"line_number":318,"context_line":"            stats_string \u003d json.dumps(stats)"},{"line_number":319,"context_line":"        except (ValueError, TypeError):"},{"line_number":320,"context_line":"            self.logger.exception(_(\u0027Error serializing stats\u0027))"},{"line_number":321,"context_line":"            return"},{"line_number":322,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"3ac371cc_cb3537f8","line":319,"in_reply_to":"9ad45d7e_c1b38a4b","updated":"2016-08-15 10:55:59.000000000","message":"Done. ValueError might raised if allow_nan is false, which is not the case here","commit_id":"beffc5014c01ee62cecefe447122912129418155"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"505e100dd79874025a6d6ffcf516fc6a4f3ecac1","unresolved":false,"context_lines":[{"line_number":318,"context_line":"            stats_string \u003d json.dumps(stats)"},{"line_number":319,"context_line":"        except (ValueError, TypeError):"},{"line_number":320,"context_line":"            self.logger.exception(_(\u0027Error serializing stats\u0027))"},{"line_number":321,"context_line":"            return"},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"        try:"},{"line_number":324,"context_line":"            with os.fdopen(self.write_fd, \u0027w\u0027) as f:"}],"source_content_type":"text/x-python","patch_set":28,"id":"9ad45d7e_681fed09","line":321,"updated":"2016-08-12 17:18:07.000000000","message":"If we return here then nothing is written to the pipe so the parent reader will hang forever. Instead, write an empty line to pipe which will cause the parent read to log an error from json.loads","commit_id":"beffc5014c01ee62cecefe447122912129418155"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"b2d0923c9b7e6b2d377c7da5ea5b7722dc242123","unresolved":false,"context_lines":[{"line_number":318,"context_line":"            stats_string \u003d json.dumps(stats)"},{"line_number":319,"context_line":"        except (ValueError, TypeError):"},{"line_number":320,"context_line":"            self.logger.exception(_(\u0027Error serializing stats\u0027))"},{"line_number":321,"context_line":"            return"},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"        try:"},{"line_number":324,"context_line":"            with os.fdopen(self.write_fd, \u0027w\u0027) as f:"}],"source_content_type":"text/x-python","patch_set":28,"id":"3ac371cc_8b2cdfac","line":321,"in_reply_to":"9ad45d7e_681fed09","updated":"2016-08-15 10:55:59.000000000","message":"Good catch!","commit_id":"beffc5014c01ee62cecefe447122912129418155"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"505e100dd79874025a6d6ffcf516fc6a4f3ecac1","unresolved":false,"context_lines":[{"line_number":344,"context_line":"        self._update_stats(stats)"},{"line_number":345,"context_line":""},{"line_number":346,"context_line":"    def periodic_report(self, t\u003dNone):"},{"line_number":347,"context_line":"        if not t:"},{"line_number":348,"context_line":"            t \u003d time()"},{"line_number":349,"context_line":"        if t - self.reported \u003e\u003d 3600:  # once an hour"},{"line_number":350,"context_line":"            self.report()"}],"source_content_type":"text/x-python","patch_set":28,"id":"9ad45d7e_9429ea8c","line":347,"updated":"2016-08-12 17:18:07.000000000","message":"nit: \n\n  if t is None\n\n(really just a nit because it would only matter if we had a test that mocked time and returned 0 (which some tests in swift do)\n\n(follow up)... I added a test that does mock time to use 0 ;) so will change this","commit_id":"beffc5014c01ee62cecefe447122912129418155"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"b2d0923c9b7e6b2d377c7da5ea5b7722dc242123","unresolved":false,"context_lines":[{"line_number":344,"context_line":"        self._update_stats(stats)"},{"line_number":345,"context_line":""},{"line_number":346,"context_line":"    def periodic_report(self, t\u003dNone):"},{"line_number":347,"context_line":"        if not t:"},{"line_number":348,"context_line":"            t \u003d time()"},{"line_number":349,"context_line":"        if t - self.reported \u003e\u003d 3600:  # once an hour"},{"line_number":350,"context_line":"            self.report()"}],"source_content_type":"text/x-python","patch_set":28,"id":"3ac371cc_1d8f9224","line":347,"in_reply_to":"9ad45d7e_9429ea8c","updated":"2016-08-15 10:55:59.000000000","message":"Done","commit_id":"beffc5014c01ee62cecefe447122912129418155"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"505e100dd79874025a6d6ffcf516fc6a4f3ecac1","unresolved":false,"context_lines":[{"line_number":356,"context_line":"        \"\"\""},{"line_number":357,"context_line":"        self.logger.info("},{"line_number":358,"context_line":"            _(\u0027Since time: %(time)s, synced: %(sync)s, [deletes: %(delete)s, \u0027"},{"line_number":359,"context_line":"              \u0027puts: %(put)s], skipped: %(skip)s, failed: %(fail)s\u0027),"},{"line_number":360,"context_line":"            {\u0027time\u0027: ctime(self.reported),"},{"line_number":361,"context_line":"             \u0027sync\u0027: self.stats[\u0027syncs\u0027],"},{"line_number":362,"context_line":"             \u0027delete\u0027: self.stats[\u0027deletes\u0027],"}],"source_content_type":"text/x-python","patch_set":28,"id":"9ad45d7e_1ac4f73d","line":359,"updated":"2016-08-12 17:18:07.000000000","message":"I agree with dropping the [..]\n\nI wonder if we should in fact move this format change to a separate patch and also add some more unit tests for it. That would make it more likely to be flagged up in the release changelog, in case any one is parsing this.","commit_id":"beffc5014c01ee62cecefe447122912129418155"},{"author":{"_account_id":860,"name":"Donagh McCabe","email":"donagh.mccabe@gmail.com","username":"donagh-mccabe"},"change_message_id":"b5bc751487bd961e3b7dfcd658757ddf63d7cf10","unresolved":false,"context_lines":[{"line_number":355,"context_line":"        next report."},{"line_number":356,"context_line":"        \"\"\""},{"line_number":357,"context_line":"        self.logger.info("},{"line_number":358,"context_line":"            _(\u0027Since time: %(time)s, synced: %(sync)s, [deletes: %(delete)s, \u0027"},{"line_number":359,"context_line":"              \u0027puts: %(put)s], skipped: %(skip)s, failed: %(fail)s\u0027),"},{"line_number":360,"context_line":"            {\u0027time\u0027: ctime(self.reported),"},{"line_number":361,"context_line":"             \u0027sync\u0027: self.stats[\u0027syncs\u0027],"},{"line_number":362,"context_line":"             \u0027delete\u0027: self.stats[\u0027deletes\u0027],"}],"source_content_type":"text/x-python","patch_set":28,"id":"9ad45d7e_6570f8a4","line":359,"range":{"start_line":358,"start_character":15,"end_line":359,"end_character":67},"updated":"2016-08-11 15:19:55.000000000","message":"This is makes this consistent with per-container message. That format was chosen to make it easy to scrap/parse the log. However, given that you *are* changing this message, can you drop the \"[\" and \"]\" -- they make it harder to parse.\n\nI think I\u0027m ok with changing this message - I doubt if anyone is parsing logs for this message, but I\u0027ll leave that as an open question for other reviewers.\n\nAn alternative is to also put this data in recon - no issues with backward comparability, would be a nice feature and is easy to extend.","commit_id":"beffc5014c01ee62cecefe447122912129418155"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"b2d0923c9b7e6b2d377c7da5ea5b7722dc242123","unresolved":false,"context_lines":[{"line_number":356,"context_line":"        \"\"\""},{"line_number":357,"context_line":"        self.logger.info("},{"line_number":358,"context_line":"            _(\u0027Since time: %(time)s, synced: %(sync)s, [deletes: %(delete)s, \u0027"},{"line_number":359,"context_line":"              \u0027puts: %(put)s], skipped: %(skip)s, failed: %(fail)s\u0027),"},{"line_number":360,"context_line":"            {\u0027time\u0027: ctime(self.reported),"},{"line_number":361,"context_line":"             \u0027sync\u0027: self.stats[\u0027syncs\u0027],"},{"line_number":362,"context_line":"             \u0027delete\u0027: self.stats[\u0027deletes\u0027],"}],"source_content_type":"text/x-python","patch_set":28,"id":"3ac371cc_7da04691","line":359,"in_reply_to":"9ad45d7e_1ac4f73d","updated":"2016-08-15 10:55:59.000000000","message":"Agree with dropping the [..]","commit_id":"beffc5014c01ee62cecefe447122912129418155"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"505e100dd79874025a6d6ffcf516fc6a4f3ecac1","unresolved":false,"context_lines":[{"line_number":362,"context_line":"             \u0027delete\u0027: self.stats[\u0027deletes\u0027],"},{"line_number":363,"context_line":"             \u0027put\u0027: self.stats[\u0027puts\u0027],"},{"line_number":364,"context_line":"             \u0027skip\u0027: self.stats[\u0027skips\u0027],"},{"line_number":365,"context_line":"             \u0027fail\u0027: self.stats[\u0027failures\u0027]})"},{"line_number":366,"context_line":""},{"line_number":367,"context_line":"        self.stats.clear()"},{"line_number":368,"context_line":"        self.reported \u003d time()"}],"source_content_type":"text/x-python","patch_set":28,"id":"9ad45d7e_ba24cb0c","line":365,"updated":"2016-08-12 17:18:07.000000000","message":"we can use self.stats here instead of creating a new dict","commit_id":"beffc5014c01ee62cecefe447122912129418155"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"b2d0923c9b7e6b2d377c7da5ea5b7722dc242123","unresolved":false,"context_lines":[{"line_number":362,"context_line":"             \u0027delete\u0027: self.stats[\u0027deletes\u0027],"},{"line_number":363,"context_line":"             \u0027put\u0027: self.stats[\u0027puts\u0027],"},{"line_number":364,"context_line":"             \u0027skip\u0027: self.stats[\u0027skips\u0027],"},{"line_number":365,"context_line":"             \u0027fail\u0027: self.stats[\u0027failures\u0027]})"},{"line_number":366,"context_line":""},{"line_number":367,"context_line":"        self.stats.clear()"},{"line_number":368,"context_line":"        self.reported \u003d time()"}],"source_content_type":"text/x-python","patch_set":28,"id":"3ac371cc_5de8ea7c","line":365,"in_reply_to":"9ad45d7e_ba24cb0c","updated":"2016-08-15 10:55:59.000000000","message":"Done","commit_id":"beffc5014c01ee62cecefe447122912129418155"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"542368b999e1ea5524449ead07d92cfbddc5efca","unresolved":false,"context_lines":[{"line_number":318,"context_line":"                # fork more than number of processes allocation defined"},{"line_number":319,"context_line":"                while len(self.pid2file) \u003e\u003d self.processes:"},{"line_number":320,"context_line":"                    self._wait_for_child()"},{"line_number":321,"context_line":"                self._do_fork()"},{"line_number":322,"context_line":"                if self.write_fd:  # forked child process"},{"line_number":323,"context_line":"                    try:"},{"line_number":324,"context_line":"                        signal.signal(signal.SIGTERM, signal.SIG_DFL)"}],"source_content_type":"text/x-python","patch_set":34,"id":"ba5da102_b9750dfb","line":321,"updated":"2016-11-02 14:28:31.000000000","message":"At summit Clay pointed out the python multiprocessing lib that would enable all we want to do here:\n\nhttps://docs.python.org/2/library/multiprocessing.html","commit_id":"5cfc29eec02043b02c44e6d385ee2d2e94795906"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"7addff656439dd83e17252e7900a1758d2404e94","unresolved":false,"context_lines":[{"line_number":318,"context_line":"                # fork more than number of processes allocation defined"},{"line_number":319,"context_line":"                while len(self.pid2file) \u003e\u003d self.processes:"},{"line_number":320,"context_line":"                    self._wait_for_child()"},{"line_number":321,"context_line":"                self._do_fork()"},{"line_number":322,"context_line":"                if self.write_fd:  # forked child process"},{"line_number":323,"context_line":"                    try:"},{"line_number":324,"context_line":"                        signal.signal(signal.SIGTERM, signal.SIG_DFL)"}],"source_content_type":"text/x-python","patch_set":34,"id":"3a71b18c_60502dd5","line":321,"in_reply_to":"ba5da102_b9750dfb","updated":"2016-12-11 12:46:07.000000000","message":"The processes fork design implemented here is also in use in both updater and auditor. After taking a look at refactoring container-sync to use multiprocessing lib for processes, I\u0027m concerned with the library requirement for pickle support, as well as with the affect of it on performance, the heart of this work","commit_id":"5cfc29eec02043b02c44e6d385ee2d2e94795906"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"542368b999e1ea5524449ead07d92cfbddc5efca","unresolved":false,"context_lines":[{"line_number":324,"context_line":"                        signal.signal(signal.SIGTERM, signal.SIG_DFL)"},{"line_number":325,"context_line":"                        # in child process, write stats to the pipe instead of"},{"line_number":326,"context_line":"                        # updating directly"},{"line_number":327,"context_line":"                        self.update_stats \u003d self._write_stats"},{"line_number":328,"context_line":"                        self.container_sync(path)"},{"line_number":329,"context_line":"                    finally:"},{"line_number":330,"context_line":"                        sys.exit()"}],"source_content_type":"text/x-python","patch_set":34,"id":"ba5da102_9925a90a","line":327,"updated":"2016-11-02 14:28:31.000000000","message":"At summit we discussed *why* I did this monkey patching. IIRC, I first tried to make container_sync return the stats dict for a container and then update the accumulated stats after container_sync has returned - either by calling update_stats in run_once or write_stats here.\n\nBut I found a number of unit tests that expect container_sync to update the instance stats dict before returning. So I avoided test churn by monkey patching.\n\nWe discussed that maybe we should take the hit and fix the tests, but that may be worth a separate patch (in a chain) to avoid confusing the multi-process aspect of this patch.","commit_id":"5cfc29eec02043b02c44e6d385ee2d2e94795906"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"211ca98023ab55bd7647ee800a473cf780665535","unresolved":false,"context_lines":[{"line_number":325,"context_line":"                        # in child process, write stats to the pipe instead of"},{"line_number":326,"context_line":"                        # updating directly"},{"line_number":327,"context_line":"                        self.update_stats \u003d self._write_stats"},{"line_number":328,"context_line":"                        self.container_sync(path)"},{"line_number":329,"context_line":"                    finally:"},{"line_number":330,"context_line":"                        sys.exit()"},{"line_number":331,"context_line":""}],"source_content_type":"text/x-python","patch_set":34,"id":"ba5da102_b960add0","line":328,"updated":"2016-11-02 14:15:33.000000000","message":"This call was discussed at the summit.  It was decided to have the stats returned from this call.  But this will change several unit tests, so the plan is to create a new patch with just this change (in order to avoid unit test diff noise).  Then this patch could depend on that one.","commit_id":"5cfc29eec02043b02c44e6d385ee2d2e94795906"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"5e7d6a4ac11059c571cf92e4d59a4ad515f2c9d9","unresolved":false,"context_lines":[{"line_number":327,"context_line":"                        self.update_stats \u003d self._write_stats"},{"line_number":328,"context_line":"                        self.container_sync(path)"},{"line_number":329,"context_line":"                    finally:"},{"line_number":330,"context_line":"                        sys.exit()"},{"line_number":331,"context_line":""},{"line_number":332,"context_line":"            # Done looping over synced containers."},{"line_number":333,"context_line":"            # Make sure all workers are done so that we don\u0027t start a new"}],"source_content_type":"text/x-python","patch_set":34,"id":"fa6399be_46cd8f85","line":330,"updated":"2016-10-17 19:01:41.000000000","message":"I just noticed today that there is a patch which changes the parent daemon class to use os._exit(0) instead of sys.exit().  See this change here: https://review.openstack.org/#/c/259347/8/swift/common/daemon.py\n\nI am wondering if that means that the child processes should do the same.  I am unaware of differences here, but looking into it.  And maybe we wait for that to merge - so that the parent and child are consistent, I don\u0027t know.","commit_id":"5cfc29eec02043b02c44e6d385ee2d2e94795906"}],"test/probe/test_container_sync.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0f65be2c07a6c64d39e90e5c5454dcb44f3c7ddf","unresolved":false,"context_lines":[{"line_number":141,"context_line":"            Manager([\u0027container-sync\u0027]).once()"},{"line_number":142,"context_line":"        else:"},{"line_number":143,"context_line":"            self.sync.start()"},{"line_number":144,"context_line":"            time.sleep(self.interval)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"    def test_sync_once(self):"},{"line_number":147,"context_line":"        container_suffixes \u003d self.create_source_and_dest_containers(\u0027source\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_4686a517","line":144,"updated":"2015-09-08 18:36:55.000000000","message":"hrmm.... I\u0027m surprised this isn\u0027t more racy\n\nI\u0027d encourage a change to let the tests to finish as quickly as possible, and also easily increase the timeout without making the test undesirably longer in the normal case.  e.g.\n\n  1. start daemons\n  2. check if it\u0027s all done\n  3. if not - sleep (up to timeout)\n  4. if so - success\n  5. if timeout \u0026 not success - say \"test failed after trying for X seconds\"","commit_id":"ee486b4ac60fe8e5f54373031856f8f71330d2be"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"d13015d18bc1748d5ce16b0b8e9c4347331835cc","unresolved":false,"context_lines":[{"line_number":141,"context_line":"            Manager([\u0027container-sync\u0027]).once()"},{"line_number":142,"context_line":"        else:"},{"line_number":143,"context_line":"            self.sync.start()"},{"line_number":144,"context_line":"            time.sleep(self.interval)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"    def test_sync_once(self):"},{"line_number":147,"context_line":"        container_suffixes \u003d self.create_source_and_dest_containers(\u0027source\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba15a1d1_6d895dca","line":144,"in_reply_to":"da20952f_4686a517","updated":"2015-09-12 21:29:53.000000000","message":"Good point. so I have (1) reduced the default time to shorten the test time and (2) added retries","commit_id":"ee486b4ac60fe8e5f54373031856f8f71330d2be"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0f65be2c07a6c64d39e90e5c5454dcb44f3c7ddf","unresolved":false,"context_lines":[{"line_number":177,"context_line":"    def test_sync_forever_multiple_containers_and_objects(self):"},{"line_number":178,"context_line":"        containers \u003d 2 * self.concurrency"},{"line_number":179,"context_line":"        container_suffixes \u003d self.create_source_and_dest_containers("},{"line_number":180,"context_line":"            \u0027source\u0027, \u0027dest\u0027, containers)"},{"line_number":181,"context_line":"        obj_names \u003d self.populate_source_containers(\u0027source\u0027,"},{"line_number":182,"context_line":"                                                    container_suffixes,"},{"line_number":183,"context_line":"                                                    10)"}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_06242d5e","line":180,"updated":"2015-09-08 18:36:55.000000000","message":"num_containers maybe","commit_id":"ee486b4ac60fe8e5f54373031856f8f71330d2be"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"d13015d18bc1748d5ce16b0b8e9c4347331835cc","unresolved":false,"context_lines":[{"line_number":177,"context_line":"    def test_sync_forever_multiple_containers_and_objects(self):"},{"line_number":178,"context_line":"        containers \u003d 2 * self.concurrency"},{"line_number":179,"context_line":"        container_suffixes \u003d self.create_source_and_dest_containers("},{"line_number":180,"context_line":"            \u0027source\u0027, \u0027dest\u0027, containers)"},{"line_number":181,"context_line":"        obj_names \u003d self.populate_source_containers(\u0027source\u0027,"},{"line_number":182,"context_line":"                                                    container_suffixes,"},{"line_number":183,"context_line":"                                                    10)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba15a1d1_8d8631b8","line":180,"in_reply_to":"da20952f_06242d5e","updated":"2015-09-12 21:29:53.000000000","message":"yep","commit_id":"ee486b4ac60fe8e5f54373031856f8f71330d2be"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e4a7e350a9c415f16824e51020420de29ffec3f0","unresolved":false,"context_lines":[{"line_number":121,"context_line":"                                   num):"},{"line_number":122,"context_line":"        obj_names \u003d []"},{"line_number":123,"context_line":"        for i in range(num):"},{"line_number":124,"context_line":"            obj_names.append(uuid.uuid4())"},{"line_number":125,"context_line":"        for container_suffix in container_suffixes:"},{"line_number":126,"context_line":"            source_container \u003d \u0027%s-container-%s\u0027 % (container_source_prefix,"},{"line_number":127,"context_line":"                                                    container_suffix)"}],"source_content_type":"text/x-python","patch_set":21,"id":"dada55a8_0ffcbb3c","line":124,"updated":"2016-07-21 12:20:36.000000000","message":"need to cast UUID to a str:\n\n  obj_names.append(str(uuid.uuid4()))\n\nsee comment line 133","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"4f74852fad09fa761891a9372395c6639121ec9e","unresolved":false,"context_lines":[{"line_number":121,"context_line":"                                   num):"},{"line_number":122,"context_line":"        obj_names \u003d []"},{"line_number":123,"context_line":"        for i in range(num):"},{"line_number":124,"context_line":"            obj_names.append(uuid.uuid4())"},{"line_number":125,"context_line":"        for container_suffix in container_suffixes:"},{"line_number":126,"context_line":"            source_container \u003d \u0027%s-container-%s\u0027 % (container_source_prefix,"},{"line_number":127,"context_line":"                                                    container_suffix)"}],"source_content_type":"text/x-python","patch_set":21,"id":"bacf61ea_17cbfe73","line":124,"in_reply_to":"dada55a8_0ffcbb3c","updated":"2016-08-01 13:47:20.000000000","message":"Yes, was fixed in patchset 13 of the following threads patch, but not here. Done.","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e4a7e350a9c415f16824e51020420de29ffec3f0","unresolved":false,"context_lines":[{"line_number":130,"context_line":"                                  self.token,"},{"line_number":131,"context_line":"                                  source_container,"},{"line_number":132,"context_line":"                                  obj_name,"},{"line_number":133,"context_line":"                                  \u0027%s\\n%s\u0027 % (container_suffix, obj_name))"},{"line_number":134,"context_line":"        return obj_names"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    def validate_container_sync_once(self,"}],"source_content_type":"text/x-python","patch_set":21,"id":"dada55a8_0fbe7bfe","line":133,"updated":"2016-07-21 12:20:36.000000000","message":"I see the following error which I think may be due to the change in *swiftclient* commit 9fed7ed - see comment at line 124.\n\n  swift@anc-vm-10:~/swift$ nosetests ./test/probe/test_container_sync.py:TestContainerSync.test_sync_forever_multiple_containers\ntest_sync_forever_multiple_containers (test.probe.test_container_sync.TestContainerSync) ... ERROR\n\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\nERROR: test_sync_forever_multiple_containers (test.probe.test_container_sync.TestContainerSync)\n----------------------------------------------------------------------\nTraceback (most recent call last):\n  File \"/home/swift/swift/test/probe/test_container_sync.py\", line 319, in test_sync_forever_multiple_containers\n    1)\n  File \"/home/swift/swift/test/probe/test_container_sync.py\", line 133, in populate_source_containers\n    \u0027%s\\n%s\u0027 % (container_suffix, obj_name))\n  File \"/home/swift/swiftclient/swiftclient/client.py\", line 1232, in put_object\n    path \u003d \u0027%s/%s\u0027 % (path.rstrip(\u0027/\u0027), quote(name))\n  File \"/home/swift/swiftclient/swiftclient/client.py\", line 210, in quote\n    return _quote(encode_utf8(value), safe)\n  File \"/usr/lib/python2.7/urllib.py\", line 1286, in quote\n    if not s.rstrip(safe):\nAttributeError: \u0027UUID\u0027 object has no attribute \u0027rstrip\u0027\n-------------------- \u003e\u003e begin captured stdout \u003c\u003c ---------------------","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"505e100dd79874025a6d6ffcf516fc6a4f3ecac1","unresolved":false,"context_lines":[{"line_number":313,"context_line":"    def test_sync_forever_multiple_containers(self):"},{"line_number":314,"context_line":"        # Test with number of containers \u003d\u003d processes"},{"line_number":315,"context_line":"        container_suffixes \u003d self.create_source_and_dest_containers("},{"line_number":316,"context_line":"            self.processes, \u0027source\u0027, \u0027dest\u0027)"},{"line_number":317,"context_line":"        obj_names \u003d self.populate_source_containers(\u0027source\u0027,"},{"line_number":318,"context_line":"                                                    container_suffixes,"},{"line_number":319,"context_line":"                                                    1)"}],"source_content_type":"text/x-python","patch_set":28,"id":"9ad45d7e_6c47f81d","line":316,"updated":"2016-08-12 17:18:07.000000000","message":"we could just create \u003e1 containers always regardless of the number of processes, then if processes\u003e1 there will be multiple child processes forked, but the test does not need to be parameterised by the config.","commit_id":"beffc5014c01ee62cecefe447122912129418155"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8b0f2c51f818e5179e4380c49564550936d2ff3b","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        self.sync \u003d Manager([\u0027container-sync\u0027])"},{"line_number":63,"context_line":"        self.sync_conf \u003d self.parse_conf()"},{"line_number":64,"context_line":"        self.processes \u003d int(self.sync_conf.get(\u0027processes\u0027, 4))"},{"line_number":65,"context_line":"        self.interval \u003d int(self.sync_conf.get(\u0027interval\u0027, 10))"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    def create_source_and_dest_containers(self, num,"},{"line_number":68,"context_line":"                                          container_source_prefix,"}],"source_content_type":"text/x-python","patch_set":30,"id":"3ac371cc_6ccd5d65","line":65,"updated":"2016-08-18 17:52:53.000000000","message":"if the conf is not set (i.e. default conf is used) then in the test we use 4 processes and an interval of 10, which is likely to be too short an interval. The tests were passing because a run_once was always being done before run_forever at line 170","commit_id":"d67c35cdc76822b65f06cf75d8a3bca8576bdc71"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8b0f2c51f818e5179e4380c49564550936d2ff3b","unresolved":false,"context_lines":[{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    def create_source_and_dest_containers(self, num,"},{"line_number":68,"context_line":"                                          container_source_prefix,"},{"line_number":69,"context_line":"                                          container_dest_prefix,"},{"line_number":70,"context_line":"                                          skey\u003d\u0027secret\u0027, dkey\u003d\u0027secret\u0027):"},{"line_number":71,"context_line":"        # setup dest containers"},{"line_number":72,"context_line":"        container_suffixes \u003d list()"}],"source_content_type":"text/x-python","patch_set":30,"id":"3ac371cc_1532faa6","line":69,"updated":"2016-08-18 17:52:53.000000000","message":"the callers always use the same prefixes so we can just hard code them","commit_id":"d67c35cdc76822b65f06cf75d8a3bca8576bdc71"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8b0f2c51f818e5179e4380c49564550936d2ff3b","unresolved":false,"context_lines":[{"line_number":146,"context_line":"                obj_body \u003d \u0027%s\\n%s\u0027 % (container_suffix, obj_name)"},{"line_number":147,"context_line":"                if body !\u003d obj_body:"},{"line_number":148,"context_line":"                    return False"},{"line_number":149,"context_line":"        return True"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"    def validate_container_sync(self,"},{"line_number":152,"context_line":"                                container_dest_prefix,"}],"source_content_type":"text/x-python","patch_set":30,"id":"3ac371cc_27cc2dba","line":149,"updated":"2016-08-18 17:52:53.000000000","message":"this method does not raise any AssertionErrors","commit_id":"d67c35cdc76822b65f06cf75d8a3bca8576bdc71"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8b0f2c51f818e5179e4380c49564550936d2ff3b","unresolved":false,"context_lines":[{"line_number":167,"context_line":"        Manager([\u0027container-replicator\u0027]).once()"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        # cycle container-sync"},{"line_number":170,"context_line":"        Manager([\u0027container-sync\u0027]).once()"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        if once is True:"},{"line_number":173,"context_line":"            Manager([\u0027container-sync\u0027]).once()"}],"source_content_type":"text/x-python","patch_set":30,"id":"3ac371cc_9bd571e6","line":170,"updated":"2016-08-18 17:52:53.000000000","message":"this should not here, see line 173","commit_id":"d67c35cdc76822b65f06cf75d8a3bca8576bdc71"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8b0f2c51f818e5179e4380c49564550936d2ff3b","unresolved":false,"context_lines":[{"line_number":269,"context_line":"        self.sync_containers(True)"},{"line_number":270,"context_line":"        self.validate_container_sync_once(\u0027dest\u0027,"},{"line_number":271,"context_line":"                                          container_suffixes,"},{"line_number":272,"context_line":"                                          obj_names)"},{"line_number":273,"context_line":""},{"line_number":274,"context_line":"    def test_sync_lazy_dkey(self):"},{"line_number":275,"context_line":"        container_suffixes \u003d self.create_source_and_dest_containers(1,"}],"source_content_type":"text/x-python","patch_set":30,"id":"3ac371cc_a7b81d12","line":272,"updated":"2016-08-18 17:52:53.000000000","message":"no assertions are made","commit_id":"d67c35cdc76822b65f06cf75d8a3bca8576bdc71"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8b0f2c51f818e5179e4380c49564550936d2ff3b","unresolved":false,"context_lines":[{"line_number":300,"context_line":"                                          container_suffixes,"},{"line_number":301,"context_line":"                                          obj_names)"},{"line_number":302,"context_line":""},{"line_number":303,"context_line":"    def test_sync_once(self):"},{"line_number":304,"context_line":"        container_suffixes \u003d self.create_source_and_dest_containers(1,"},{"line_number":305,"context_line":"                                                                    \u0027source\u0027,"},{"line_number":306,"context_line":"                                                                    \u0027dest\u0027)"}],"source_content_type":"text/x-python","patch_set":30,"id":"3ac371cc_1d3d00dd","line":303,"updated":"2016-08-18 17:52:53.000000000","message":"this duplicates _test_sync","commit_id":"d67c35cdc76822b65f06cf75d8a3bca8576bdc71"}],"test/unit/container/test_replicator.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0f65be2c07a6c64d39e90e5c5454dcb44f3c7ddf","unresolved":false,"context_lines":[{"line_number":1002,"context_line":"        add_sync \u003d [0]"},{"line_number":1003,"context_line":"        remove_sync \u003d [0]"},{"line_number":1004,"context_line":""},{"line_number":1005,"context_line":"        class FakeContainerSyncStore(object):"},{"line_number":1006,"context_line":""},{"line_number":1007,"context_line":"            def add_synced_container(self, broker):"},{"line_number":1008,"context_line":"                add_sync[0] +\u003d 1"}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_86b7dd39","line":1005,"updated":"2015-09-08 18:36:55.000000000","message":"sure, probably a good idea.  Sometimes these sorts of minor cleanups look better in a separate patch where they\u0027re not distracting from the meat of the change.  Although in *this* case it only takes all of like 1 second to see \"oh yeah sure, obviously\" - sometimes what appears at first like a \"quick cleanup\" can actually result in a very distracting bike shedding session.  As long as you continue to show good judgement about what \"should obviously be fixed\" it won\u0027t be an issue - just something to keep in mind.","commit_id":"ee486b4ac60fe8e5f54373031856f8f71330d2be"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"d13015d18bc1748d5ce16b0b8e9c4347331835cc","unresolved":false,"context_lines":[{"line_number":1002,"context_line":"        add_sync \u003d [0]"},{"line_number":1003,"context_line":"        remove_sync \u003d [0]"},{"line_number":1004,"context_line":""},{"line_number":1005,"context_line":"        class FakeContainerSyncStore(object):"},{"line_number":1006,"context_line":""},{"line_number":1007,"context_line":"            def add_synced_container(self, broker):"},{"line_number":1008,"context_line":"                add_sync[0] +\u003d 1"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba15a1d1_2d9365fc","line":1005,"in_reply_to":"da20952f_86b7dd39","updated":"2015-09-12 21:29:53.000000000","message":"I will keep that in mind (I hope that the stats change, however, does not prove that I am not...)\nanyways with this one I think pep8 would complain.","commit_id":"ee486b4ac60fe8e5f54373031856f8f71330d2be"}],"test/unit/container/test_sync.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0f65be2c07a6c64d39e90e5c5454dcb44f3c7ddf","unresolved":false,"context_lines":[{"line_number":268,"context_line":"        orig_fork \u003d sync.os.fork"},{"line_number":269,"context_line":"        orig_wait \u003d sync.os.wait"},{"line_number":270,"context_line":"        orig_pipe \u003d sync.os.pipe"},{"line_number":271,"context_line":"        orig_exit \u003d sync.sys.exit"},{"line_number":272,"context_line":""},{"line_number":273,"context_line":"        patch_sync()"},{"line_number":274,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_063f4d88","line":271,"updated":"2015-09-08 18:36:55.000000000","message":"I think this would look better with mock instead of the \"old-school\" manual patch - but maybe it doesn\u0027t matter.  Cleanup in code that you\u0027re already working in for the root of the change is almost always welcome.","commit_id":"ee486b4ac60fe8e5f54373031856f8f71330d2be"},{"author":{"_account_id":11317,"name":"Eran Rom","email":"eran@itsonlyme.name","username":"eranr"},"change_message_id":"d13015d18bc1748d5ce16b0b8e9c4347331835cc","unresolved":false,"context_lines":[{"line_number":268,"context_line":"        orig_fork \u003d sync.os.fork"},{"line_number":269,"context_line":"        orig_wait \u003d sync.os.wait"},{"line_number":270,"context_line":"        orig_pipe \u003d sync.os.pipe"},{"line_number":271,"context_line":"        orig_exit \u003d sync.sys.exit"},{"line_number":272,"context_line":""},{"line_number":273,"context_line":"        patch_sync()"},{"line_number":274,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba15a1d1_4d9019ed","line":271,"in_reply_to":"da20952f_063f4d88","updated":"2015-09-12 21:29:53.000000000","message":"yep, moved to mock.","commit_id":"ee486b4ac60fe8e5f54373031856f8f71330d2be"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e4a7e350a9c415f16824e51020420de29ffec3f0","unresolved":false,"context_lines":[{"line_number":115,"context_line":"            self.assertTrue(expected in line, msg)"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"    @with_tempdir"},{"line_number":118,"context_line":"    def test_init(self, tempdir):"},{"line_number":119,"context_line":"        ic_conf_path \u003d os.path.join(tempdir, \u0027internal-client.conf\u0027)"},{"line_number":120,"context_line":"        cring \u003d FakeRing()"},{"line_number":121,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"dada55a8_950b9e7c","line":118,"updated":"2016-07-21 12:20:36.000000000","message":"tests needed for the new config option","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e4a7e350a9c415f16824e51020420de29ffec3f0","unresolved":false,"context_lines":[{"line_number":171,"context_line":"        # With multi-processing the collection of container sync stats is"},{"line_number":172,"context_line":"        # done via pipes. Each child process that works on a container"},{"line_number":173,"context_line":"        # gets a write end of a pipe, and the parent gets the read end"},{"line_number":174,"context_line":"        # of all spawned processses. In the unit tests fake the process"},{"line_number":175,"context_line":"        # spawning. the way the code is written this force us to supply"},{"line_number":176,"context_line":"        # the pipes from the test code."},{"line_number":177,"context_line":"        pipes \u003d [os.pipe() for i in range(3)]"}],"source_content_type":"text/x-python","patch_set":21,"id":"dada55a8_15204ef6","line":174,"updated":"2016-07-21 12:20:36.000000000","message":"nit s/processses/processes/","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"4f74852fad09fa761891a9372395c6639121ec9e","unresolved":false,"context_lines":[{"line_number":171,"context_line":"        # With multi-processing the collection of container sync stats is"},{"line_number":172,"context_line":"        # done via pipes. Each child process that works on a container"},{"line_number":173,"context_line":"        # gets a write end of a pipe, and the parent gets the read end"},{"line_number":174,"context_line":"        # of all spawned processses. In the unit tests fake the process"},{"line_number":175,"context_line":"        # spawning. the way the code is written this force us to supply"},{"line_number":176,"context_line":"        # the pipes from the test code."},{"line_number":177,"context_line":"        pipes \u003d [os.pipe() for i in range(3)]"}],"source_content_type":"text/x-python","patch_set":21,"id":"bacf61ea_17a01e90","line":174,"in_reply_to":"dada55a8_15204ef6","updated":"2016-08-01 13:47:20.000000000","message":"Done","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e4a7e350a9c415f16824e51020420de29ffec3f0","unresolved":false,"context_lines":[{"line_number":216,"context_line":""},{"line_number":217,"context_line":"            # Assert here on stats as they are being zeroed"},{"line_number":218,"context_line":"            # at reports. This tests the pipe mechanism for passing"},{"line_number":219,"context_line":"            # the stats correctly."},{"line_number":220,"context_line":"            if time_calls[0] \u003d\u003d 2:"},{"line_number":221,"context_line":"                self.assertEqual(stats[\u0027syncs\u0027], 1)"},{"line_number":222,"context_line":"            if time_calls[0] \u003d\u003d 3:"}],"source_content_type":"text/x-python","patch_set":21,"id":"dada55a8_f58dbad3","line":219,"updated":"2016-07-21 12:20:36.000000000","message":"I think I\u0027d prefer to use a FakeLogger and make assertion that the expected lines are logged. I\u0027m nervous when assertions are made deep in a mocked function - it is possible that an \"except Exception\" somewhere in the code would obscure any assertion errors.","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"4f74852fad09fa761891a9372395c6639121ec9e","unresolved":false,"context_lines":[{"line_number":216,"context_line":""},{"line_number":217,"context_line":"            # Assert here on stats as they are being zeroed"},{"line_number":218,"context_line":"            # at reports. This tests the pipe mechanism for passing"},{"line_number":219,"context_line":"            # the stats correctly."},{"line_number":220,"context_line":"            if time_calls[0] \u003d\u003d 2:"},{"line_number":221,"context_line":"                self.assertEqual(stats[\u0027syncs\u0027], 1)"},{"line_number":222,"context_line":"            if time_calls[0] \u003d\u003d 3:"}],"source_content_type":"text/x-python","patch_set":21,"id":"bacf61ea_aa736b14","line":219,"in_reply_to":"dada55a8_f58dbad3","updated":"2016-08-01 13:47:20.000000000","message":"Done","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e4a7e350a9c415f16824e51020420de29ffec3f0","unresolved":false,"context_lines":[{"line_number":241,"context_line":"        gf \u003d sc + \u0027sync_store.ContainerSyncStore.synced_containers_generator\u0027"},{"line_number":242,"context_line":"        scscs \u003d scs + \u0027ContainerSync.container_sync\u0027"},{"line_number":243,"context_line":"        with mock.patch(scs + \u0027InternalClient\u0027):"},{"line_number":244,"context_line":"            cs \u003d sync.ContainerSync({}, container_ring\u003dFakeRing())"},{"line_number":245,"context_line":"        with mock.patch(scs + \u0027time\u0027, fake_time), \\"},{"line_number":246,"context_line":"                mock.patch(scs + \u0027sleep\u0027) as fake_sleep, \\"},{"line_number":247,"context_line":"                mock.patch(scs + \u0027os.fork\u0027, fake_fork), \\"}],"source_content_type":"text/x-python","patch_set":21,"id":"dada55a8_f5a49a4b","line":244,"updated":"2016-07-21 12:20:36.000000000","message":"pass a FakeLogger to ContainerSync","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"4f74852fad09fa761891a9372395c6639121ec9e","unresolved":false,"context_lines":[{"line_number":241,"context_line":"        gf \u003d sc + \u0027sync_store.ContainerSyncStore.synced_containers_generator\u0027"},{"line_number":242,"context_line":"        scscs \u003d scs + \u0027ContainerSync.container_sync\u0027"},{"line_number":243,"context_line":"        with mock.patch(scs + \u0027InternalClient\u0027):"},{"line_number":244,"context_line":"            cs \u003d sync.ContainerSync({}, container_ring\u003dFakeRing())"},{"line_number":245,"context_line":"        with mock.patch(scs + \u0027time\u0027, fake_time), \\"},{"line_number":246,"context_line":"                mock.patch(scs + \u0027sleep\u0027) as fake_sleep, \\"},{"line_number":247,"context_line":"                mock.patch(scs + \u0027os.fork\u0027, fake_fork), \\"}],"source_content_type":"text/x-python","patch_set":21,"id":"bacf61ea_0a61d739","line":244,"in_reply_to":"dada55a8_f5a49a4b","updated":"2016-08-01 13:47:20.000000000","message":"Done","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e4a7e350a9c415f16824e51020420de29ffec3f0","unresolved":false,"context_lines":[{"line_number":242,"context_line":"        scscs \u003d scs + \u0027ContainerSync.container_sync\u0027"},{"line_number":243,"context_line":"        with mock.patch(scs + \u0027InternalClient\u0027):"},{"line_number":244,"context_line":"            cs \u003d sync.ContainerSync({}, container_ring\u003dFakeRing())"},{"line_number":245,"context_line":"        with mock.patch(scs + \u0027time\u0027, fake_time), \\"},{"line_number":246,"context_line":"                mock.patch(scs + \u0027sleep\u0027) as fake_sleep, \\"},{"line_number":247,"context_line":"                mock.patch(scs + \u0027os.fork\u0027, fake_fork), \\"},{"line_number":248,"context_line":"                mock.patch(scs + \u0027os.wait\u0027, fake_wait), \\"}],"source_content_type":"text/x-python","patch_set":21,"id":"dada55a8_3550f231","line":245,"updated":"2016-07-21 12:20:36.000000000","message":"do we no longer need to mock InternalClient?","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"4f74852fad09fa761891a9372395c6639121ec9e","unresolved":false,"context_lines":[{"line_number":242,"context_line":"        scscs \u003d scs + \u0027ContainerSync.container_sync\u0027"},{"line_number":243,"context_line":"        with mock.patch(scs + \u0027InternalClient\u0027):"},{"line_number":244,"context_line":"            cs \u003d sync.ContainerSync({}, container_ring\u003dFakeRing())"},{"line_number":245,"context_line":"        with mock.patch(scs + \u0027time\u0027, fake_time), \\"},{"line_number":246,"context_line":"                mock.patch(scs + \u0027sleep\u0027) as fake_sleep, \\"},{"line_number":247,"context_line":"                mock.patch(scs + \u0027os.fork\u0027, fake_fork), \\"},{"line_number":248,"context_line":"                mock.patch(scs + \u0027os.wait\u0027, fake_wait), \\"}],"source_content_type":"text/x-python","patch_set":21,"id":"bacf61ea_0ac8b721","line":245,"in_reply_to":"dada55a8_3550f231","updated":"2016-08-01 13:47:20.000000000","message":"Looks ok to me","commit_id":"26e175e4177f18abb397bba3fb02a5c4d7202c07"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"94218bc21abbb0d90c509e290ac21d9a97f96dea","unresolved":false,"context_lines":[{"line_number":233,"context_line":"        def fake_sync(self, path):"},{"line_number":234,"context_line":"            sync_calls[0] +\u003d 1"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"        def fake_write_stats(self, fd):"},{"line_number":237,"context_line":"            stats \u003d collections.defaultdict(int)"},{"line_number":238,"context_line":"            i \u003d sync_calls[0]"},{"line_number":239,"context_line":"            if i \u003d\u003d 1:"}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_bd576925","line":236,"updated":"2016-08-03 18:10:06.000000000","message":"this fake method is not needed, fake_sync is passed a ref to the ContainerSync instance (as arg \u0027self\u0027) so you can update self.stats in that method.","commit_id":"b06a315c3ab3b2cdf96e4feacdcfaef21feabea5"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"94218bc21abbb0d90c509e290ac21d9a97f96dea","unresolved":false,"context_lines":[{"line_number":255,"context_line":"        sc \u003d \u0027swift.container.\u0027"},{"line_number":256,"context_line":"        scs \u003d sc + \u0027sync.\u0027"},{"line_number":257,"context_line":"        gf \u003d sc + \u0027sync_store.ContainerSyncStore.synced_containers_generator\u0027"},{"line_number":258,"context_line":"        scscs \u003d scs + \u0027ContainerSync.container_sync\u0027"},{"line_number":259,"context_line":"        with mock.patch(scs + \u0027InternalClient\u0027):"},{"line_number":260,"context_line":"            cs \u003d sync.ContainerSync({}, container_ring\u003dFakeRing(),"},{"line_number":261,"context_line":"                                    logger\u003dFakeLogger())"}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_22db291d","line":258,"updated":"2016-08-03 18:10:06.000000000","message":"nit: I find the string variable construction harder to read than having them explicit and split across lines in the mock statements","commit_id":"b06a315c3ab3b2cdf96e4feacdcfaef21feabea5"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"94218bc21abbb0d90c509e290ac21d9a97f96dea","unresolved":false,"context_lines":[{"line_number":258,"context_line":"        scscs \u003d scs + \u0027ContainerSync.container_sync\u0027"},{"line_number":259,"context_line":"        with mock.patch(scs + \u0027InternalClient\u0027):"},{"line_number":260,"context_line":"            cs \u003d sync.ContainerSync({}, container_ring\u003dFakeRing(),"},{"line_number":261,"context_line":"                                    logger\u003dFakeLogger())"},{"line_number":262,"context_line":"            g_cs \u003d cs"},{"line_number":263,"context_line":"        with mock.patch(scs + \u0027time\u0027, fake_time), \\"},{"line_number":264,"context_line":"                mock.patch(scs + \u0027sleep\u0027) as fake_sleep, \\"}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_2cf162ce","line":261,"updated":"2016-08-03 18:10:06.000000000","message":"I\u0027d prefer all the mocking to be done before the instance is created.","commit_id":"b06a315c3ab3b2cdf96e4feacdcfaef21feabea5"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"94218bc21abbb0d90c509e290ac21d9a97f96dea","unresolved":false,"context_lines":[{"line_number":270,"context_line":"                mock.patch(scs + \u0027sys.exit\u0027, fake_exit), \\"},{"line_number":271,"context_line":"                mock.patch(scscs, fake_sync), \\"},{"line_number":272,"context_line":"                mock.patch(gf) as fake_generator, \\"},{"line_number":273,"context_line":"                mock.patch(scs + \u0027ContainerBroker\u0027,"},{"line_number":274,"context_line":"                           lambda p: FakeContainerBroker(p, info\u003d{"},{"line_number":275,"context_line":"                               \u0027account\u0027: \u0027a\u0027, \u0027container\u0027: \u0027c\u0027,"},{"line_number":276,"context_line":"                               \u0027storage_policy_index\u0027: 0})):"}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_00fe0c05","line":273,"updated":"2016-08-03 18:10:06.000000000","message":"now that the container_sync method is mocked there is no need to mock ContainerBroker - it is never used.","commit_id":"b06a315c3ab3b2cdf96e4feacdcfaef21feabea5"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"94218bc21abbb0d90c509e290ac21d9a97f96dea","unresolved":false,"context_lines":[{"line_number":276,"context_line":"                               \u0027storage_policy_index\u0027: 0})):"},{"line_number":277,"context_line":"            fake_generator.side_effect \u003d [iter([\u0027container.db\u0027]),"},{"line_number":278,"context_line":"                                          iter([\u0027container.db\u0027]),"},{"line_number":279,"context_line":"                                          iter(\u0027container.db\u0027)]"},{"line_number":280,"context_line":"            cs.pid2file \u003d pid2file"},{"line_number":281,"context_line":"            cs.reported \u003d 0"},{"line_number":282,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_bfe72b3f","line":279,"updated":"2016-08-03 18:10:06.000000000","message":"should be an iter of a list i.e. iter([\u0027container.db\u0027]","commit_id":"b06a315c3ab3b2cdf96e4feacdcfaef21feabea5"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"94218bc21abbb0d90c509e290ac21d9a97f96dea","unresolved":false,"context_lines":[{"line_number":288,"context_line":"            log_lines \u003d cs.logger.get_lines_for_level(\u0027info\u0027)"},{"line_number":289,"context_line":"            self.assertTrue(\"synced: 1, [deletes: 0, puts: 0], skipped: 0,\""},{"line_number":290,"context_line":"                            \" failed: 0\" in"},{"line_number":291,"context_line":"                            log_lines[2])"},{"line_number":292,"context_line":"            self.assertTrue(\"synced: 0, [deletes: 1, puts: 0], skipped: 0,\""},{"line_number":293,"context_line":"                            \" failed: 0\" in"},{"line_number":294,"context_line":"                            log_lines[4])"}],"source_content_type":"text/x-python","patch_set":24,"id":"bacf61ea_da4e6ac0","line":291,"updated":"2016-08-03 18:10:06.000000000","message":"I was expecting to see synced\u003d2, deletes\u003d1 because by the time we report, there have been two sweeps. BUT the stats for second sweep have not bee read from the pipe when the report is made. This is, to a degree, a side-effect of the test mocking out the forking. But it is confusing.","commit_id":"b06a315c3ab3b2cdf96e4feacdcfaef21feabea5"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"505e100dd79874025a6d6ffcf516fc6a4f3ecac1","unresolved":false,"context_lines":[{"line_number":317,"context_line":"                    self.assertEqual(10, fake_time.call_count)"},{"line_number":318,"context_line":"                    self.assertEqual(2, fake_generator.call_count)"},{"line_number":319,"context_line":"                    self.assertEqual(cs.reported, 3605)"},{"line_number":320,"context_line":"                except Exception as err:"},{"line_number":321,"context_line":"                    if str(err) !\u003d \u0027we are now done\u0027:"},{"line_number":322,"context_line":"                        raise"},{"line_number":323,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"9ad45d7e_a7a62b51","line":320,"updated":"2016-08-12 17:18:07.000000000","message":"we don\u0027t need the sentinel Exception for run_once","commit_id":"beffc5014c01ee62cecefe447122912129418155"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"505e100dd79874025a6d6ffcf516fc6a4f3ecac1","unresolved":false,"context_lines":[{"line_number":319,"context_line":"                    self.assertEqual(cs.reported, 3605)"},{"line_number":320,"context_line":"                except Exception as err:"},{"line_number":321,"context_line":"                    if str(err) !\u003d \u0027we are now done\u0027:"},{"line_number":322,"context_line":"                        raise"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"    def test_container_sync_not_db(self):"},{"line_number":325,"context_line":"        cring \u003d FakeRing()"}],"source_content_type":"text/x-python","patch_set":28,"id":"9ad45d7e_27719be1","line":322,"updated":"2016-08-12 17:18:07.000000000","message":"great cleanup of this test, thanks","commit_id":"beffc5014c01ee62cecefe447122912129418155"},{"author":{"_account_id":3094,"name":"oshritf","email":"oshritf@il.ibm.com","username":"oshritf"},"change_message_id":"b2d0923c9b7e6b2d377c7da5ea5b7722dc242123","unresolved":false,"context_lines":[{"line_number":319,"context_line":"                    self.assertEqual(cs.reported, 3605)"},{"line_number":320,"context_line":"                except Exception as err:"},{"line_number":321,"context_line":"                    if str(err) !\u003d \u0027we are now done\u0027:"},{"line_number":322,"context_line":"                        raise"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"    def test_container_sync_not_db(self):"},{"line_number":325,"context_line":"        cring \u003d FakeRing()"}],"source_content_type":"text/x-python","patch_set":28,"id":"3ac371cc_7d8686ba","line":322,"in_reply_to":"9ad45d7e_27719be1","updated":"2016-08-15 10:55:59.000000000","message":":)","commit_id":"beffc5014c01ee62cecefe447122912129418155"}]}