)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"dcba13186f034081033829f772236d42d3e7ab7a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"9384d06c_f4af172f","updated":"2026-01-14 19:56:13.000000000","message":"Hey Dan, Thank you for inputs. I have added replies inline for your comments.","commit_id":"ef1c360dcec1cfed75c3dadc66612571ab99b84d"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"953af4b1341782b86ba96e3e9c0d4e13d3bd0bef","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"84ec2948_3e6a3b84","updated":"2026-01-15 06:40:28.000000000","message":"Hi Dan, thank you for review!!","commit_id":"ef1c360dcec1cfed75c3dadc66612571ab99b84d"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"d3fae0b1fc8a81920ac31f6fb5f71964145b2b50","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"eda9b565_cbcd36b5","updated":"2026-01-15 06:42:06.000000000","message":"NOTE: Also need changes in glance to handle TimeoutError raised form glance_store.","commit_id":"ef1c360dcec1cfed75c3dadc66612571ab99b84d"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"0328334e1eaf78dadf10920794e5489727ace95d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9d274334_53d59334","updated":"2026-01-15 17:38:04.000000000","message":"Thank you for inputs Dan, working on to fixing them.","commit_id":"a1a097dfc2ba05fbc9b3408fbdd781d473fbc53f"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"9b493a299defd90679f1ca0cd414c6b98315dc92","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"d855cc48_114baa34","updated":"2026-01-19 05:49:13.000000000","message":"hi Dan, \nThank you for review, I have added two more tests as suggested.","commit_id":"81b1f459467cff946927392820b50dd32094b96f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e0f5a7d887d21d60d3c2ef122049e8ac93989b91","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"db7fac36_c1524b6b","updated":"2026-01-29 16:13:11.000000000","message":"Abhi, very sorry I just realized we never finished this. Just a couple more comments on the tests (with my apologies about the second one) and I\u0027m good otherwise.","commit_id":"1c00db0c56a46e32070580f40bfdb512420d243c"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"74c72fba9fd0a129f53547207e03f127cd4361e1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"f7c0abcf_37df6a7d","updated":"2026-01-29 17:38:59.000000000","message":"Hi Dan, No worries, thank you for your reviews on this. I have modified the test as per your suggestion.","commit_id":"1c00db0c56a46e32070580f40bfdb512420d243c"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"4a1a2a01b214b0642fba05eaaf666e22b7a7f6fd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"0ce1ab3c_cf46f4cc","updated":"2026-02-03 20:37:29.000000000","message":"Looks aligned with the spec, thanks!","commit_id":"42847e6c0f7f08e46d9cb163a55d8283e6559913"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"318336b1b3b48986a5270e9de0053d731384064e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"d534470b_fabee145","updated":"2026-02-04 13:30:59.000000000","message":"recheck","commit_id":"42847e6c0f7f08e46d9cb163a55d8283e6559913"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"ab01836b5d1788902c2fe0a5333c40eb55a09303","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"42941206_7ed3f822","updated":"2026-02-04 08:02:46.000000000","message":"recheck unrelated timeout","commit_id":"42847e6c0f7f08e46d9cb163a55d8283e6559913"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"3fa25af8ba89f78743e46280a8e770733c9f1813","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"bb30aebd_559d75de","updated":"2026-02-04 04:24:49.000000000","message":"recheck volume error","commit_id":"42847e6c0f7f08e46d9cb163a55d8283e6559913"}],"glance_store/_drivers/filesystem.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ccb655b7d78fc4857055d0f0ebd1f43175f83328","unresolved":true,"context_lines":[{"line_number":350,"context_line":"            self.fp \u003d None"},{"line_number":351,"context_line":""},{"line_number":352,"context_line":""},{"line_number":353,"context_line":"class TimeoutExecutor(object):"},{"line_number":354,"context_line":"    def __init__(self, timeout, pool_size, threshold):"},{"line_number":355,"context_line":"        self.timeout \u003d timeout"},{"line_number":356,"context_line":"        self.pool_size \u003d pool_size"}],"source_content_type":"text/x-python","patch_set":1,"id":"5680147e_a691c7f0","line":353,"range":{"start_line":353,"start_character":22,"end_line":353,"end_character":29},"updated":"2026-01-14 19:20:06.000000000","message":"This isn\u0027t python2 :)","commit_id":"ef1c360dcec1cfed75c3dadc66612571ab99b84d"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"dcba13186f034081033829f772236d42d3e7ab7a","unresolved":false,"context_lines":[{"line_number":350,"context_line":"            self.fp \u003d None"},{"line_number":351,"context_line":""},{"line_number":352,"context_line":""},{"line_number":353,"context_line":"class TimeoutExecutor(object):"},{"line_number":354,"context_line":"    def __init__(self, timeout, pool_size, threshold):"},{"line_number":355,"context_line":"        self.timeout \u003d timeout"},{"line_number":356,"context_line":"        self.pool_size \u003d pool_size"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f4a80af_78b82bcb","line":353,"range":{"start_line":353,"start_character":22,"end_line":353,"end_character":29},"in_reply_to":"5680147e_a691c7f0","updated":"2026-01-14 19:56:13.000000000","message":"Acknowledged, just followed the same way as class ChunkedFile(object): at line #306","commit_id":"ef1c360dcec1cfed75c3dadc66612571ab99b84d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ccb655b7d78fc4857055d0f0ebd1f43175f83328","unresolved":true,"context_lines":[{"line_number":359,"context_line":"        self.lock \u003d threading.Lock()"},{"line_number":360,"context_line":"        self.active_futures \u003d set()"},{"line_number":361,"context_line":"        if timeout \u003e 0:"},{"line_number":362,"context_line":"            self.executor \u003d futures.ThreadPoolExecutor(max_workers\u003dpool_size)"},{"line_number":363,"context_line":""},{"line_number":364,"context_line":"    def execute(self, func, *args, **kwargs):"},{"line_number":365,"context_line":"        if self.timeout \u003d\u003d 0 or self.executor is None:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f71da28_142df9cd","line":362,"updated":"2026-01-14 19:20:06.000000000","message":"Why not use futurist and its SynchronousExecutor here (if timeout\u003d\u003d0)? Then the rest of the implementation is identical.","commit_id":"ef1c360dcec1cfed75c3dadc66612571ab99b84d"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"dcba13186f034081033829f772236d42d3e7ab7a","unresolved":false,"context_lines":[{"line_number":359,"context_line":"        self.lock \u003d threading.Lock()"},{"line_number":360,"context_line":"        self.active_futures \u003d set()"},{"line_number":361,"context_line":"        if timeout \u003e 0:"},{"line_number":362,"context_line":"            self.executor \u003d futures.ThreadPoolExecutor(max_workers\u003dpool_size)"},{"line_number":363,"context_line":""},{"line_number":364,"context_line":"    def execute(self, func, *args, **kwargs):"},{"line_number":365,"context_line":"        if self.timeout \u003d\u003d 0 or self.executor is None:"}],"source_content_type":"text/x-python","patch_set":1,"id":"d2c81743_73fba660","line":362,"in_reply_to":"9f71da28_142df9cd","updated":"2026-01-14 19:56:13.000000000","message":"Ack, will make changes accordingly.","commit_id":"ef1c360dcec1cfed75c3dadc66612571ab99b84d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ccb655b7d78fc4857055d0f0ebd1f43175f83328","unresolved":true,"context_lines":[{"line_number":365,"context_line":"        if self.timeout \u003d\u003d 0 or self.executor is None:"},{"line_number":366,"context_line":"            return func(*args, **kwargs)"},{"line_number":367,"context_line":""},{"line_number":368,"context_line":"        future \u003d self.executor.submit(func, *args, **kwargs)"},{"line_number":369,"context_line":"        with self.lock:"},{"line_number":370,"context_line":"            self.active_futures.add(future)"},{"line_number":371,"context_line":"            # Check usage AFTER adding the current future"}],"source_content_type":"text/x-python","patch_set":1,"id":"6233444f_caf0d4a9","line":368,"updated":"2026-01-14 19:20:06.000000000","message":"This will block if you\u0027re out of threads right? I thought it would, but now that I\u0027m looking at the API docs I don\u0027t see a statement one way or the other.","commit_id":"ef1c360dcec1cfed75c3dadc66612571ab99b84d"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"dcba13186f034081033829f772236d42d3e7ab7a","unresolved":false,"context_lines":[{"line_number":365,"context_line":"        if self.timeout \u003d\u003d 0 or self.executor is None:"},{"line_number":366,"context_line":"            return func(*args, **kwargs)"},{"line_number":367,"context_line":""},{"line_number":368,"context_line":"        future \u003d self.executor.submit(func, *args, **kwargs)"},{"line_number":369,"context_line":"        with self.lock:"},{"line_number":370,"context_line":"            self.active_futures.add(future)"},{"line_number":371,"context_line":"            # Check usage AFTER adding the current future"}],"source_content_type":"text/x-python","patch_set":1,"id":"13c3e93d_07f0de5d","line":368,"in_reply_to":"6233444f_caf0d4a9","updated":"2026-01-14 19:56:13.000000000","message":"it returns immediately and queues the task if threads are busy.","commit_id":"ef1c360dcec1cfed75c3dadc66612571ab99b84d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ccb655b7d78fc4857055d0f0ebd1f43175f83328","unresolved":true,"context_lines":[{"line_number":366,"context_line":"            return func(*args, **kwargs)"},{"line_number":367,"context_line":""},{"line_number":368,"context_line":"        future \u003d self.executor.submit(func, *args, **kwargs)"},{"line_number":369,"context_line":"        with self.lock:"},{"line_number":370,"context_line":"            self.active_futures.add(future)"},{"line_number":371,"context_line":"            # Check usage AFTER adding the current future"},{"line_number":372,"context_line":"            active \u003d len([f for f in self.active_futures if not f.done()])"}],"source_content_type":"text/x-python","patch_set":1,"id":"95a71510_98d10688","line":369,"updated":"2026-01-14 19:20:06.000000000","message":"I\u0027m not sure this lock is really necessary, especially since it\u0027s just for warning.. Are you just trying to be extra good here or is there something you\u0027re addressing specifically?","commit_id":"ef1c360dcec1cfed75c3dadc66612571ab99b84d"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"953af4b1341782b86ba96e3e9c0d4e13d3bd0bef","unresolved":false,"context_lines":[{"line_number":366,"context_line":"            return func(*args, **kwargs)"},{"line_number":367,"context_line":""},{"line_number":368,"context_line":"        future \u003d self.executor.submit(func, *args, **kwargs)"},{"line_number":369,"context_line":"        with self.lock:"},{"line_number":370,"context_line":"            self.active_futures.add(future)"},{"line_number":371,"context_line":"            # Check usage AFTER adding the current future"},{"line_number":372,"context_line":"            active \u003d len([f for f in self.active_futures if not f.done()])"}],"source_content_type":"text/x-python","patch_set":1,"id":"bc59fe6a_99c3bfed","line":369,"in_reply_to":"1905114b_8016cd58","updated":"2026-01-15 06:40:28.000000000","message":"Makes sense, removed the lock.","commit_id":"ef1c360dcec1cfed75c3dadc66612571ab99b84d"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"dcba13186f034081033829f772236d42d3e7ab7a","unresolved":true,"context_lines":[{"line_number":366,"context_line":"            return func(*args, **kwargs)"},{"line_number":367,"context_line":""},{"line_number":368,"context_line":"        future \u003d self.executor.submit(func, *args, **kwargs)"},{"line_number":369,"context_line":"        with self.lock:"},{"line_number":370,"context_line":"            self.active_futures.add(future)"},{"line_number":371,"context_line":"            # Check usage AFTER adding the current future"},{"line_number":372,"context_line":"            active \u003d len([f for f in self.active_futures if not f.done()])"}],"source_content_type":"text/x-python","patch_set":1,"id":"b29182f7_f7df2628","line":369,"in_reply_to":"95a71510_98d10688","updated":"2026-01-14 19:56:13.000000000","message":"I thought we need to protect add/discard operations from concurrent access by threads to get accurate usage calculation.","commit_id":"ef1c360dcec1cfed75c3dadc66612571ab99b84d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"740861c4437bcacc20adec6c24ab23d312463349","unresolved":true,"context_lines":[{"line_number":366,"context_line":"            return func(*args, **kwargs)"},{"line_number":367,"context_line":""},{"line_number":368,"context_line":"        future \u003d self.executor.submit(func, *args, **kwargs)"},{"line_number":369,"context_line":"        with self.lock:"},{"line_number":370,"context_line":"            self.active_futures.add(future)"},{"line_number":371,"context_line":"            # Check usage AFTER adding the current future"},{"line_number":372,"context_line":"            active \u003d len([f for f in self.active_futures if not f.done()])"}],"source_content_type":"text/x-python","patch_set":1,"id":"1905114b_8016cd58","line":369,"in_reply_to":"b29182f7_f7df2628","updated":"2026-01-14 21:03:12.000000000","message":"Well, the thing is, this is just for a warning so the math being exact is not that important.. if one attempt doesn\u0027t get it due to a race, the next likely will. Point is that using a lock when you don\u0027t need one increases the potential for a deadlock. Your LOG message below will need to generate IO, which could block while holding the lock, preventing other things from discarding from the set and returning, etc. Since this is just a \"meh, things are looking pretty busy around here\" sort of check, letting it be slightly less deterministic seems better to me.","commit_id":"ef1c360dcec1cfed75c3dadc66612571ab99b84d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"50560a12b6ec96c1c07325dad0df19502ca9d980","unresolved":true,"context_lines":[{"line_number":362,"context_line":""},{"line_number":363,"context_line":"    def execute(self, func, *args, **kwargs):"},{"line_number":364,"context_line":"        future \u003d self.executor.submit(func, *args, **kwargs)"},{"line_number":365,"context_line":"        if self.timeout \u003e 0:"},{"line_number":366,"context_line":"            self.active_futures.add(future)"},{"line_number":367,"context_line":"            # Check usage AFTER adding the current future"},{"line_number":368,"context_line":"            active \u003d len([f for f in self.active_futures if not f.done()])"}],"source_content_type":"text/x-python","patch_set":2,"id":"db880f40_8071ea97","line":365,"updated":"2026-01-15 16:20:58.000000000","message":"I guess I had envisioned just not having this \"if timeout\" condition anywhere after this, just to make it cleaner to read. I guess you\u0027re doing this just to avoid running the (would be) no-op code below?\n\nYou could also change this to `if not future.done()` which would handle the synchronous case always, as well as the case where we schedule a task and it completes before we even wait for it.\n\nAnyway, just nit talk, not suggesting a need to change if you prefer this.","commit_id":"a1a097dfc2ba05fbc9b3408fbdd781d473fbc53f"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"0328334e1eaf78dadf10920794e5489727ace95d","unresolved":false,"context_lines":[{"line_number":362,"context_line":""},{"line_number":363,"context_line":"    def execute(self, func, *args, **kwargs):"},{"line_number":364,"context_line":"        future \u003d self.executor.submit(func, *args, **kwargs)"},{"line_number":365,"context_line":"        if self.timeout \u003e 0:"},{"line_number":366,"context_line":"            self.active_futures.add(future)"},{"line_number":367,"context_line":"            # Check usage AFTER adding the current future"},{"line_number":368,"context_line":"            active \u003d len([f for f in self.active_futures if not f.done()])"}],"source_content_type":"text/x-python","patch_set":2,"id":"d3a0f4bf_939bc16c","line":365,"in_reply_to":"db880f40_8071ea97","updated":"2026-01-15 17:38:04.000000000","message":"Ack, changing it to use not future.done()","commit_id":"a1a097dfc2ba05fbc9b3408fbdd781d473fbc53f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"50560a12b6ec96c1c07325dad0df19502ca9d980","unresolved":true,"context_lines":[{"line_number":366,"context_line":"            self.active_futures.add(future)"},{"line_number":367,"context_line":"            # Check usage AFTER adding the current future"},{"line_number":368,"context_line":"            active \u003d len([f for f in self.active_futures if not f.done()])"},{"line_number":369,"context_line":"            if self.pool_size \u003e 0:"},{"line_number":370,"context_line":"                usage \u003d (active / self.pool_size) * 100"},{"line_number":371,"context_line":"            else:"},{"line_number":372,"context_line":"                usage \u003d 0"}],"source_content_type":"text/x-python","patch_set":2,"id":"09966d96_a78d7467","line":369,"updated":"2026-01-15 16:20:58.000000000","message":"AFAICT, this can never happen because it comes straight from the conf option and that has `min\u003d1`. That makes L372 dead code.","commit_id":"a1a097dfc2ba05fbc9b3408fbdd781d473fbc53f"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"0328334e1eaf78dadf10920794e5489727ace95d","unresolved":false,"context_lines":[{"line_number":366,"context_line":"            self.active_futures.add(future)"},{"line_number":367,"context_line":"            # Check usage AFTER adding the current future"},{"line_number":368,"context_line":"            active \u003d len([f for f in self.active_futures if not f.done()])"},{"line_number":369,"context_line":"            if self.pool_size \u003e 0:"},{"line_number":370,"context_line":"                usage \u003d (active / self.pool_size) * 100"},{"line_number":371,"context_line":"            else:"},{"line_number":372,"context_line":"                usage \u003d 0"}],"source_content_type":"text/x-python","patch_set":2,"id":"a37465f5_c65bb8e3","line":369,"in_reply_to":"09966d96_a78d7467","updated":"2026-01-15 17:38:04.000000000","message":"You are right!!","commit_id":"a1a097dfc2ba05fbc9b3408fbdd781d473fbc53f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"50560a12b6ec96c1c07325dad0df19502ca9d980","unresolved":true,"context_lines":[{"line_number":375,"context_line":"                                \"(threshold: %(threshold)d%%). Pool may \""},{"line_number":376,"context_line":"                                \"start blocking.\") %"},{"line_number":377,"context_line":"                            {\u0027usage\u0027: usage,"},{"line_number":378,"context_line":"                             \u0027threshold\u0027: self.threshold})"},{"line_number":379,"context_line":"        try:"},{"line_number":380,"context_line":"            return future.result(timeout\u003dself.timeout)"},{"line_number":381,"context_line":"        except futures.TimeoutError:"}],"source_content_type":"text/x-python","patch_set":2,"id":"42ecacc9_305b7242","line":378,"updated":"2026-01-15 16:20:58.000000000","message":"This never gets run in the tests. Might be best to have a dedicated test class for just unit-testing this utility. If you use something like a `threading.Event` or `threading.Condition` you can spin up `N` threads waiting for that, ensure you hit this and do the log, then set the condition to unblock the threads, reap them and finish the test.\n\nI can cook something up if you\u0027re not sure about all that, just let me know.","commit_id":"a1a097dfc2ba05fbc9b3408fbdd781d473fbc53f"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"0328334e1eaf78dadf10920794e5489727ace95d","unresolved":false,"context_lines":[{"line_number":375,"context_line":"                                \"(threshold: %(threshold)d%%). Pool may \""},{"line_number":376,"context_line":"                                \"start blocking.\") %"},{"line_number":377,"context_line":"                            {\u0027usage\u0027: usage,"},{"line_number":378,"context_line":"                             \u0027threshold\u0027: self.threshold})"},{"line_number":379,"context_line":"        try:"},{"line_number":380,"context_line":"            return future.result(timeout\u003dself.timeout)"},{"line_number":381,"context_line":"        except futures.TimeoutError:"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa62b79_6fb99d4f","line":378,"in_reply_to":"42ecacc9_305b7242","updated":"2026-01-15 17:38:04.000000000","message":"Ack, will try to write a separate test for TimeoutExecutor","commit_id":"a1a097dfc2ba05fbc9b3408fbdd781d473fbc53f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"50560a12b6ec96c1c07325dad0df19502ca9d980","unresolved":true,"context_lines":[{"line_number":381,"context_line":"        except futures.TimeoutError:"},{"line_number":382,"context_line":"            raise exceptions.TimeoutError(timeout\u003dself.timeout)"},{"line_number":383,"context_line":"        finally:"},{"line_number":384,"context_line":"            if self.timeout \u003e 0:"},{"line_number":385,"context_line":"                self.active_futures.discard(future)"},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"    def shutdown(self, wait\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":2,"id":"f0e9ce8a_ea682df1","line":384,"updated":"2026-01-15 16:20:58.000000000","message":"Even if you keep the \"if timeout\" above, `discard()` will not raise if the future isn\u0027t in the set, so you can just remove this and always call the next statement.","commit_id":"a1a097dfc2ba05fbc9b3408fbdd781d473fbc53f"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"0328334e1eaf78dadf10920794e5489727ace95d","unresolved":false,"context_lines":[{"line_number":381,"context_line":"        except futures.TimeoutError:"},{"line_number":382,"context_line":"            raise exceptions.TimeoutError(timeout\u003dself.timeout)"},{"line_number":383,"context_line":"        finally:"},{"line_number":384,"context_line":"            if self.timeout \u003e 0:"},{"line_number":385,"context_line":"                self.active_futures.discard(future)"},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"    def shutdown(self, wait\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":2,"id":"c73ebacd_90f01ea2","line":384,"in_reply_to":"f0e9ce8a_ea682df1","updated":"2026-01-15 17:38:04.000000000","message":"Acknowledged","commit_id":"a1a097dfc2ba05fbc9b3408fbdd781d473fbc53f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"50560a12b6ec96c1c07325dad0df19502ca9d980","unresolved":true,"context_lines":[{"line_number":385,"context_line":"                self.active_futures.discard(future)"},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"    def shutdown(self, wait\u003dTrue):"},{"line_number":388,"context_line":"        self.executor.shutdown(wait\u003dwait)"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"class Store(glance_store.driver.Store):"}],"source_content_type":"text/x-python","patch_set":2,"id":"b94dc507_f5760f1c","line":388,"updated":"2026-01-15 16:20:58.000000000","message":"Also never run in the tests, but if you fix the threshold thing above, this will be part of the test to finish I think.","commit_id":"a1a097dfc2ba05fbc9b3408fbdd781d473fbc53f"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"0328334e1eaf78dadf10920794e5489727ace95d","unresolved":false,"context_lines":[{"line_number":385,"context_line":"                self.active_futures.discard(future)"},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"    def shutdown(self, wait\u003dTrue):"},{"line_number":388,"context_line":"        self.executor.shutdown(wait\u003dwait)"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"class Store(glance_store.driver.Store):"}],"source_content_type":"text/x-python","patch_set":2,"id":"e994bf9f_3c445d75","line":388,"in_reply_to":"b94dc507_f5760f1c","updated":"2026-01-15 17:38:04.000000000","message":"Acknowledged","commit_id":"a1a097dfc2ba05fbc9b3408fbdd781d473fbc53f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"50560a12b6ec96c1c07325dad0df19502ca9d980","unresolved":true,"context_lines":[{"line_number":672,"context_line":"        if not self.timeout_executor.execute(os.path.exists, filepath):"},{"line_number":673,"context_line":"            raise exceptions.NotFound(image\u003dfilepath)"},{"line_number":674,"context_line":""},{"line_number":675,"context_line":"        filesize \u003d self.timeout_executor.execute(os.path.getsize, filepath)"},{"line_number":676,"context_line":"        return filepath, filesize"},{"line_number":677,"context_line":""},{"line_number":678,"context_line":"    def _get_metadata(self, filepath):"}],"source_content_type":"text/x-python","patch_set":2,"id":"aa9d24b1_893835c1","line":675,"updated":"2026-01-15 16:20:58.000000000","message":"So, this function will create a thread, let it run, wait for it, then create another thread, let it run, wait for it. But I don\u0027t think we need to do that. First, `os.path.getsize()` will raise `FileNotFound` if it\u0027s missing, which we can catch and return `NotFound` instead of doing two steps.\n\nSecond, if we did (or do, for some reason) need to do both, it would be better to just exec `_resolve_location()` in the executor.. one thing to do two things in the worker thread, return/raise one result.","commit_id":"a1a097dfc2ba05fbc9b3408fbdd781d473fbc53f"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"0328334e1eaf78dadf10920794e5489727ace95d","unresolved":false,"context_lines":[{"line_number":672,"context_line":"        if not self.timeout_executor.execute(os.path.exists, filepath):"},{"line_number":673,"context_line":"            raise exceptions.NotFound(image\u003dfilepath)"},{"line_number":674,"context_line":""},{"line_number":675,"context_line":"        filesize \u003d self.timeout_executor.execute(os.path.getsize, filepath)"},{"line_number":676,"context_line":"        return filepath, filesize"},{"line_number":677,"context_line":""},{"line_number":678,"context_line":"    def _get_metadata(self, filepath):"}],"source_content_type":"text/x-python","patch_set":2,"id":"67810d63_01332af7","line":675,"in_reply_to":"aa9d24b1_893835c1","updated":"2026-01-15 17:38:04.000000000","message":"Ack, refactored to use a single executor call.","commit_id":"a1a097dfc2ba05fbc9b3408fbdd781d473fbc53f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"50560a12b6ec96c1c07325dad0df19502ca9d980","unresolved":true,"context_lines":[{"line_number":763,"context_line":"        if self.timeout_executor.execute(os.path.exists, fn):"},{"line_number":764,"context_line":"            try:"},{"line_number":765,"context_line":"                LOG.debug(_(\"Deleting image at %(fn)s\"), {\u0027fn\u0027: fn})"},{"line_number":766,"context_line":"                self.timeout_executor.execute(os.unlink, fn)"},{"line_number":767,"context_line":"            except OSError:"},{"line_number":768,"context_line":"                raise exceptions.Forbidden("},{"line_number":769,"context_line":"                    message\u003d(_(\"You cannot delete file %s\") % fn))"}],"source_content_type":"text/x-python","patch_set":2,"id":"6941062c_3915e49f","line":766,"updated":"2026-01-15 16:20:58.000000000","message":"Same here, we can just exec `unlink` and handle `OSError` and `FileNotFound` separately right?","commit_id":"a1a097dfc2ba05fbc9b3408fbdd781d473fbc53f"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"0328334e1eaf78dadf10920794e5489727ace95d","unresolved":false,"context_lines":[{"line_number":763,"context_line":"        if self.timeout_executor.execute(os.path.exists, fn):"},{"line_number":764,"context_line":"            try:"},{"line_number":765,"context_line":"                LOG.debug(_(\"Deleting image at %(fn)s\"), {\u0027fn\u0027: fn})"},{"line_number":766,"context_line":"                self.timeout_executor.execute(os.unlink, fn)"},{"line_number":767,"context_line":"            except OSError:"},{"line_number":768,"context_line":"                raise exceptions.Forbidden("},{"line_number":769,"context_line":"                    message\u003d(_(\"You cannot delete file %s\") % fn))"}],"source_content_type":"text/x-python","patch_set":2,"id":"a38edd1d_87110100","line":766,"in_reply_to":"6941062c_3915e49f","updated":"2026-01-15 17:38:04.000000000","message":"Acknowledged","commit_id":"a1a097dfc2ba05fbc9b3408fbdd781d473fbc53f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"256ed573e5ec32f1c71cd992356db764333f9275","unresolved":false,"context_lines":[{"line_number":360,"context_line":"        else:"},{"line_number":361,"context_line":"            self.executor \u003d futurist.SynchronousExecutor()"},{"line_number":362,"context_line":""},{"line_number":363,"context_line":"    def execute(self, func, *args, **kwargs):"},{"line_number":364,"context_line":"        future \u003d self.executor.submit(func, *args, **kwargs)"},{"line_number":365,"context_line":"        if not future.done():"},{"line_number":366,"context_line":"            self.active_futures.add(future)"}],"source_content_type":"text/x-python","patch_set":3,"id":"f60daefa_205cf1cd","line":363,"updated":"2026-01-16 18:06:35.000000000","message":"This looks much tighter now...","commit_id":"81b1f459467cff946927392820b50dd32094b96f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"256ed573e5ec32f1c71cd992356db764333f9275","unresolved":true,"context_lines":[{"line_number":764,"context_line":"            raise exceptions.NotFound(image\u003dfn)"},{"line_number":765,"context_line":"        except OSError:"},{"line_number":766,"context_line":"            raise exceptions.Forbidden("},{"line_number":767,"context_line":"                message\u003d(_(\"You cannot delete file %s\") % fn))"},{"line_number":768,"context_line":""},{"line_number":769,"context_line":"    def _get_capacity_info(self, mount_point):"},{"line_number":770,"context_line":"        \"\"\"Calculates total available space for given mount point."}],"source_content_type":"text/x-python","patch_set":3,"id":"e9ce2d1c_ba917953","line":767,"updated":"2026-01-16 18:06:35.000000000","message":"This and the above are much nicer I think 😊\n\nI\u0027d also note that this fixes a race that exists in the code, if the file was deleted between checking `exists()` and actually doing it. It\u0027s a very small window, but it\u0027s there :)","commit_id":"81b1f459467cff946927392820b50dd32094b96f"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"9b493a299defd90679f1ca0cd414c6b98315dc92","unresolved":false,"context_lines":[{"line_number":764,"context_line":"            raise exceptions.NotFound(image\u003dfn)"},{"line_number":765,"context_line":"        except OSError:"},{"line_number":766,"context_line":"            raise exceptions.Forbidden("},{"line_number":767,"context_line":"                message\u003d(_(\"You cannot delete file %s\") % fn))"},{"line_number":768,"context_line":""},{"line_number":769,"context_line":"    def _get_capacity_info(self, mount_point):"},{"line_number":770,"context_line":"        \"\"\"Calculates total available space for given mount point."}],"source_content_type":"text/x-python","patch_set":3,"id":"92fd6c78_b284751d","line":767,"in_reply_to":"e9ce2d1c_ba917953","updated":"2026-01-19 05:49:13.000000000","message":"Yes!!","commit_id":"81b1f459467cff946927392820b50dd32094b96f"}],"glance_store/tests/unit/test_filesystem_store.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"256ed573e5ec32f1c71cd992356db764333f9275","unresolved":true,"context_lines":[{"line_number":393,"context_line":"    \"\"\"Test TimeoutExecutor utility class\"\"\""},{"line_number":394,"context_line":""},{"line_number":395,"context_line":"    def setUp(self):"},{"line_number":396,"context_line":"        super(TestTimeoutExecutor, self).setUp()"},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"    def test_thread_pool_threshold_warning(self):"},{"line_number":399,"context_line":"        \"\"\"Test that warning is logged when thread pool usage exceeds"}],"source_content_type":"text/x-python","patch_set":3,"id":"7fbc059a_f9da44d6","line":396,"updated":"2026-01-16 18:06:35.000000000","message":"Could remove this if you\u0027re not going to use it for anything else.","commit_id":"81b1f459467cff946927392820b50dd32094b96f"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"9b493a299defd90679f1ca0cd414c6b98315dc92","unresolved":false,"context_lines":[{"line_number":393,"context_line":"    \"\"\"Test TimeoutExecutor utility class\"\"\""},{"line_number":394,"context_line":""},{"line_number":395,"context_line":"    def setUp(self):"},{"line_number":396,"context_line":"        super(TestTimeoutExecutor, self).setUp()"},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"    def test_thread_pool_threshold_warning(self):"},{"line_number":399,"context_line":"        \"\"\"Test that warning is logged when thread pool usage exceeds"}],"source_content_type":"text/x-python","patch_set":3,"id":"c9e9cbed_ee026bf0","line":396,"in_reply_to":"7fbc059a_f9da44d6","updated":"2026-01-19 05:49:13.000000000","message":"Acknowledged","commit_id":"81b1f459467cff946927392820b50dd32094b96f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"256ed573e5ec32f1c71cd992356db764333f9275","unresolved":true,"context_lines":[{"line_number":433,"context_line":"            for t in threads:"},{"line_number":434,"context_line":"                t.join(timeout\u003d5)"},{"line_number":435,"context_line":""},{"line_number":436,"context_line":"        executor.shutdown(wait\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":3,"id":"47f19c06_cbb4d1a9","line":436,"updated":"2026-01-16 18:06:35.000000000","message":"Can we add another similar case that actually tests the timeout behavior? Set the timeout very low, block the threads similarly and make sure they timeout cleanly?\n\nI also think one where the pool is consumed on blocked threads, make sure \u003epool_size threads wait for timeout and then gracefully raise and don\u0027t get stuck.","commit_id":"81b1f459467cff946927392820b50dd32094b96f"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"9b493a299defd90679f1ca0cd414c6b98315dc92","unresolved":false,"context_lines":[{"line_number":433,"context_line":"            for t in threads:"},{"line_number":434,"context_line":"                t.join(timeout\u003d5)"},{"line_number":435,"context_line":""},{"line_number":436,"context_line":"        executor.shutdown(wait\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":3,"id":"cf5aaac0_3b63f256","line":436,"in_reply_to":"47f19c06_cbb4d1a9","updated":"2026-01-19 05:49:13.000000000","message":"Done","commit_id":"81b1f459467cff946927392820b50dd32094b96f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e0f5a7d887d21d60d3c2ef122049e8ac93989b91","unresolved":true,"context_lines":[{"line_number":458,"context_line":""},{"line_number":459,"context_line":"        time.sleep(0.05)"},{"line_number":460,"context_line":"        self.assertRaises(exceptions.TimeoutError,"},{"line_number":461,"context_line":"                          executor.execute, blocking_func)"},{"line_number":462,"context_line":""},{"line_number":463,"context_line":"        event.set()"},{"line_number":464,"context_line":"        for t in threads:"}],"source_content_type":"text/x-python","patch_set":4,"id":"2dd98406_47725757","line":461,"updated":"2026-01-29 16:13:11.000000000","message":"I think actually for this you don\u0027t need the other threads and can just test that one thing waiting on something that will never return is enough right? Also, I think your 50ms sleep is doing nothing here, right? The `blocking_func` will block regardless of if the other threads have started right?","commit_id":"1c00db0c56a46e32070580f40bfdb512420d243c"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"74c72fba9fd0a129f53547207e03f127cd4361e1","unresolved":false,"context_lines":[{"line_number":458,"context_line":""},{"line_number":459,"context_line":"        time.sleep(0.05)"},{"line_number":460,"context_line":"        self.assertRaises(exceptions.TimeoutError,"},{"line_number":461,"context_line":"                          executor.execute, blocking_func)"},{"line_number":462,"context_line":""},{"line_number":463,"context_line":"        event.set()"},{"line_number":464,"context_line":"        for t in threads:"}],"source_content_type":"text/x-python","patch_set":4,"id":"d6872a61_398bf052","line":461,"in_reply_to":"2dd98406_47725757","updated":"2026-01-29 17:38:59.000000000","message":"You are right, modifying the test!","commit_id":"1c00db0c56a46e32070580f40bfdb512420d243c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e0f5a7d887d21d60d3c2ef122049e8ac93989b91","unresolved":true,"context_lines":[{"line_number":494,"context_line":""},{"line_number":495,"context_line":"        for i in range(5):"},{"line_number":496,"context_line":"            self.assertRaises(exceptions.TimeoutError,"},{"line_number":497,"context_line":"                              executor.execute, blocking_func)"},{"line_number":498,"context_line":""},{"line_number":499,"context_line":"        event.set()"},{"line_number":500,"context_line":"        for t in threads:"}],"source_content_type":"text/x-python","patch_set":4,"id":"80082d2f_76c6d7ef","line":497,"updated":"2026-01-29 16:13:11.000000000","message":"Can we distinguish this from the case above? Point being, I\u0027m not convinced this is actually timing out in 0.1s but rather blocking until some of the threads consuming the pool return to free up space. I know I asked for this, but now that I\u0027m seeing it and refreshing my memory on how `executor.submit()` works, I think this is just hanging until one of the above threads finishes, then running the will-never-return function five times until it times out, right? Meaning, I don\u0027t think this is actually testing that `submit()` times out (because it doesn\u0027t, it hangs) and the loop of five is just doing the same thing over and over.\n\nSo, apologies, but I guess this is not a useful test after all.","commit_id":"1c00db0c56a46e32070580f40bfdb512420d243c"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"74c72fba9fd0a129f53547207e03f127cd4361e1","unresolved":false,"context_lines":[{"line_number":494,"context_line":""},{"line_number":495,"context_line":"        for i in range(5):"},{"line_number":496,"context_line":"            self.assertRaises(exceptions.TimeoutError,"},{"line_number":497,"context_line":"                              executor.execute, blocking_func)"},{"line_number":498,"context_line":""},{"line_number":499,"context_line":"        event.set()"},{"line_number":500,"context_line":"        for t in threads:"}],"source_content_type":"text/x-python","patch_set":4,"id":"4a43438b_0e284c7b","line":497,"in_reply_to":"80082d2f_76c6d7ef","updated":"2026-01-29 17:38:59.000000000","message":"Removed the test. you are right, executor.execute() blocks when the pool is full rather than timing out.","commit_id":"1c00db0c56a46e32070580f40bfdb512420d243c"}]}