)]}'
{"taskflow/conductors/backends/impl_executor.py":[{"author":{"_account_id":5164,"name":"jsimms","email":"justin.simms@gmail.com","username":"jsimms"},"change_message_id":"5277deed9fd4ed1e88cf82e6e0d65c2cc8a9ea36","unresolved":false,"context_lines":[{"line_number":268,"context_line":"                        self._log.debug(\"Trying to claim job: %s\", job)"},{"line_number":269,"context_line":"                        try:"},{"line_number":270,"context_line":"                            self._jobboard.claim(job, self._name)"},{"line_number":271,"context_line":"                        except (excp.UnclaimableJob, excp.NotFound):"},{"line_number":272,"context_line":"                            self._log.debug(\"Job already claimed or\""},{"line_number":273,"context_line":"                                            \" consumed: %s\", job)"},{"line_number":274,"context_line":"                        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a26ad4f_988dfa7d","line":271,"updated":"2015-10-28 03:00:10.000000000","message":"Why did you decide to combine the two exception types in this case, instead of leaving them as their own exception paths with clear logging that indicates the state of the job?","commit_id":"3a8dac62ad854aa35343798e69beb96b1acfd81e"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"55a2bd568c80d4315ba2d2fd09e470067015d463","unresolved":false,"context_lines":[{"line_number":268,"context_line":"                        self._log.debug(\"Trying to claim job: %s\", job)"},{"line_number":269,"context_line":"                        try:"},{"line_number":270,"context_line":"                            self._jobboard.claim(job, self._name)"},{"line_number":271,"context_line":"                        except (excp.UnclaimableJob, excp.NotFound):"},{"line_number":272,"context_line":"                            self._log.debug(\"Job already claimed or\""},{"line_number":273,"context_line":"                                            \" consumed: %s\", job)"},{"line_number":274,"context_line":"                        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a26ad4f_d755801d","line":271,"in_reply_to":"1a26ad4f_988dfa7d","updated":"2015-10-28 05:15:28.000000000","message":"Could do that, but thats the current state of the world, this patch was just trying to change the concurrent dispatch limiting mechanism, not this part.","commit_id":"3a8dac62ad854aa35343798e69beb96b1acfd81e"},{"author":{"_account_id":5164,"name":"jsimms","email":"justin.simms@gmail.com","username":"jsimms"},"change_message_id":"5277deed9fd4ed1e88cf82e6e0d65c2cc8a9ea36","unresolved":false,"context_lines":[{"line_number":272,"context_line":"                            self._log.debug(\"Job already claimed or\""},{"line_number":273,"context_line":"                                            \" consumed: %s\", job)"},{"line_number":274,"context_line":"                        else:"},{"line_number":275,"context_line":"                            try:"},{"line_number":276,"context_line":"                                fut \u003d executor.submit(self._dispatch_job, job)"},{"line_number":277,"context_line":"                            except RuntimeError:"},{"line_number":278,"context_line":"                                with excutils.save_and_reraise_exception():"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a26ad4f_58876261","line":275,"updated":"2015-10-28 03:00:10.000000000","message":"Why did you decide to separate the submission of the job from attempting to claim it? It feels like putting the two together would simplify this logic.","commit_id":"3a8dac62ad854aa35343798e69beb96b1acfd81e"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"55a2bd568c80d4315ba2d2fd09e470067015d463","unresolved":false,"context_lines":[{"line_number":272,"context_line":"                            self._log.debug(\"Job already claimed or\""},{"line_number":273,"context_line":"                                            \" consumed: %s\", job)"},{"line_number":274,"context_line":"                        else:"},{"line_number":275,"context_line":"                            try:"},{"line_number":276,"context_line":"                                fut \u003d executor.submit(self._dispatch_job, job)"},{"line_number":277,"context_line":"                            except RuntimeError:"},{"line_number":278,"context_line":"                                with excutils.save_and_reraise_exception():"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a26ad4f_f70c3c44","line":275,"in_reply_to":"1a26ad4f_58876261","updated":"2015-10-28 05:15:28.000000000","message":"Avoids wasting executor threads on jobs that aren\u0027t going to be used in the first place; central claim then dispatch seems simpler imho, but maybe thats just my style, either could be done of course.","commit_id":"3a8dac62ad854aa35343798e69beb96b1acfd81e"},{"author":{"_account_id":5164,"name":"jsimms","email":"justin.simms@gmail.com","username":"jsimms"},"change_message_id":"9f28838affb68a542fb7e2b9b61a1a2dfb7a66da","unresolved":false,"context_lines":[{"line_number":260,"context_line":"            while not is_stopped():"},{"line_number":261,"context_line":"                any_dispatched \u003d False"},{"line_number":262,"context_line":"                for job in self._jobboard.iterjobs():"},{"line_number":263,"context_line":"                    if is_stopped() or not self._limiter.acquire():"},{"line_number":264,"context_line":"                        break"},{"line_number":265,"context_line":"                    delayed_release \u003d False"},{"line_number":266,"context_line":"                    try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a26ad4f_93907dee","line":263,"updated":"2015-10-28 04:12:23.000000000","message":"Was self._limiter.release() left out on purpose?","commit_id":"de87b7d06a22fe408dfcf39d5e866f14e8992a82"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"10af33b543e359532cbf87e5c9d5e17364e960b8","unresolved":false,"context_lines":[{"line_number":260,"context_line":"            while not is_stopped():"},{"line_number":261,"context_line":"                any_dispatched \u003d False"},{"line_number":262,"context_line":"                for job in self._jobboard.iterjobs():"},{"line_number":263,"context_line":"                    if is_stopped() or not self._limiter.acquire():"},{"line_number":264,"context_line":"                        break"},{"line_number":265,"context_line":"                    delayed_release \u003d False"},{"line_number":266,"context_line":"                    try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a26ad4f_d77ee0ad","line":263,"in_reply_to":"1a26ad4f_93907dee","updated":"2015-10-28 05:12:53.000000000","message":"See next patch, this changed so that its more obvious whats happening here, but yes, acquire didn\u0027t work out, so no release needed.","commit_id":"de87b7d06a22fe408dfcf39d5e866f14e8992a82"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"5e93958c123229f80aaa95ef77f0ff153537d674","unresolved":false,"context_lines":[{"line_number":260,"context_line":"            while not is_stopped():"},{"line_number":261,"context_line":"                any_dispatched \u003d False"},{"line_number":262,"context_line":"                for job in self._jobboard.iterjobs():"},{"line_number":263,"context_line":"                    if is_stopped() or not self._limiter.acquire():"},{"line_number":264,"context_line":"                        break"},{"line_number":265,"context_line":"                    delayed_release \u003d False"},{"line_number":266,"context_line":"                    try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a26ad4f_3ab6198b","line":263,"in_reply_to":"1a26ad4f_ba778901","updated":"2015-10-28 06:09:15.000000000","message":"Right, that shouldn\u0027t be possible, hopefully the followup where a exception is raised for this case now makes that more obvious.","commit_id":"de87b7d06a22fe408dfcf39d5e866f14e8992a82"},{"author":{"_account_id":5164,"name":"jsimms","email":"justin.simms@gmail.com","username":"jsimms"},"change_message_id":"8f4ac2c6138c0a9265aa35dc4201eaf0a33fdd14","unresolved":false,"context_lines":[{"line_number":260,"context_line":"            while not is_stopped():"},{"line_number":261,"context_line":"                any_dispatched \u003d False"},{"line_number":262,"context_line":"                for job in self._jobboard.iterjobs():"},{"line_number":263,"context_line":"                    if is_stopped() or not self._limiter.acquire():"},{"line_number":264,"context_line":"                        break"},{"line_number":265,"context_line":"                    delayed_release \u003d False"},{"line_number":266,"context_line":"                    try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a26ad4f_ba778901","line":263,"in_reply_to":"1a26ad4f_d77ee0ad","updated":"2015-10-28 06:02:49.000000000","message":"Ok, cool. I was thinking along the lines of the self._limiter.acquire() call succeeding at this point, but there not being a release once the submission completed.","commit_id":"de87b7d06a22fe408dfcf39d5e866f14e8992a82"},{"author":{"_account_id":5164,"name":"jsimms","email":"justin.simms@gmail.com","username":"jsimms"},"change_message_id":"3ef47108fb385eac2ae28899004901decf669edf","unresolved":false,"context_lines":[{"line_number":260,"context_line":"                raise StopIteration"},{"line_number":261,"context_line":"            while not is_stopped():"},{"line_number":262,"context_line":"                any_dispatched \u003d False"},{"line_number":263,"context_line":"                for job in itertools.takewhile(lambda job: not is_stopped(),"},{"line_number":264,"context_line":"                                               self._jobboard.iterjobs()):"},{"line_number":265,"context_line":"                    self._limiter.acquire()"},{"line_number":266,"context_line":"                    delayed_release \u003d False"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a26ad4f_faf8a17e","line":263,"updated":"2015-10-28 05:59:05.000000000","message":"So I can see that you are iterating through the jobs on the job board, but the lambda expression seems a little odd to me since is_stopped is not making use of the job parameter. What am I missing here?","commit_id":"d2a065e51bacb805a92837291feeaacf43147969"},{"author":{"_account_id":5164,"name":"jsimms","email":"justin.simms@gmail.com","username":"jsimms"},"change_message_id":"371e5804d5a94c62642ca8100ca728c6408b15d1","unresolved":false,"context_lines":[{"line_number":260,"context_line":"                raise StopIteration"},{"line_number":261,"context_line":"            while not is_stopped():"},{"line_number":262,"context_line":"                any_dispatched \u003d False"},{"line_number":263,"context_line":"                for job in itertools.takewhile(lambda job: not is_stopped(),"},{"line_number":264,"context_line":"                                               self._jobboard.iterjobs()):"},{"line_number":265,"context_line":"                    self._limiter.acquire()"},{"line_number":266,"context_line":"                    delayed_release \u003d False"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a26ad4f_1a231dbf","line":263,"in_reply_to":"1a26ad4f_1a00fd33","updated":"2015-10-28 06:12:48.000000000","message":"Ok, I was thinking that you were using takewhile that way, so we are in sync. I figured I would just ask instead of assuming, ;\u0027) . Thanks for the clarification.","commit_id":"d2a065e51bacb805a92837291feeaacf43147969"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"1f2302200b76f34c839731e1297aae81a65bc9d6","unresolved":false,"context_lines":[{"line_number":260,"context_line":"                raise StopIteration"},{"line_number":261,"context_line":"            while not is_stopped():"},{"line_number":262,"context_line":"                any_dispatched \u003d False"},{"line_number":263,"context_line":"                for job in itertools.takewhile(lambda job: not is_stopped(),"},{"line_number":264,"context_line":"                                               self._jobboard.iterjobs()):"},{"line_number":265,"context_line":"                    self._limiter.acquire()"},{"line_number":266,"context_line":"                    delayed_release \u003d False"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a26ad4f_1a00fd33","line":263,"in_reply_to":"1a26ad4f_faf8a17e","updated":"2015-10-28 06:08:30.000000000","message":"Right, its just use as a iteration control/breaking mechanism.\n\nI can write this as:\n\n  for job in self._jobboard.iterjobs():\n     if is_stopped():\n        break\n\nEither works, what do u prefer?\n\nThis just uses itertools.takewhile to do the same thing (exception that itertools function passes the value in, which we don\u0027t care about here); https://docs.python.org/2/library/itertools.html#itertools.takewhile","commit_id":"d2a065e51bacb805a92837291feeaacf43147969"},{"author":{"_account_id":5164,"name":"jsimms","email":"justin.simms@gmail.com","username":"jsimms"},"change_message_id":"94c22fca857333126e4da6421fe77e137479074c","unresolved":false,"context_lines":[{"line_number":126,"context_line":"            event_factory\u003dself._event_factory)"},{"line_number":127,"context_line":"        self._dead \u003d self._event_factory()"},{"line_number":128,"context_line":"        self._log \u003d misc.pick_first_not_none(log, self.LOG, LOG)"},{"line_number":129,"context_line":"        max_simultaneous_jobs \u003d int(misc.pick_first_not_none("},{"line_number":130,"context_line":"            max_simultaneous_jobs, self.MAX_SIMULTANEOUS_JOBS))"},{"line_number":131,"context_line":"        self._max_simultaneous_jobs \u003d max_simultaneous_jobs"},{"line_number":132,"context_line":"        if max_simultaneous_jobs \u003e 0:"}],"source_content_type":"text/x-python","patch_set":15,"id":"1a26ad4f_79c4685e","line":129,"updated":"2015-10-29 17:03:24.000000000","message":"Would it make sense to wrap the max job integer logic in a _convert* function of some kind? Or, even better, create a _create_limiter function that wraps all of the max job logic and factory callout logic for the limiter?","commit_id":"3d62d7fd500491bb2ee0a936ac8ca6e676e43f09"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"0db4734e6c35867fd8072104dae7881d93672508","unresolved":false,"context_lines":[{"line_number":126,"context_line":"            event_factory\u003dself._event_factory)"},{"line_number":127,"context_line":"        self._dead \u003d self._event_factory()"},{"line_number":128,"context_line":"        self._log \u003d misc.pick_first_not_none(log, self.LOG, LOG)"},{"line_number":129,"context_line":"        max_simultaneous_jobs \u003d int(misc.pick_first_not_none("},{"line_number":130,"context_line":"            max_simultaneous_jobs, self.MAX_SIMULTANEOUS_JOBS))"},{"line_number":131,"context_line":"        self._max_simultaneous_jobs \u003d max_simultaneous_jobs"},{"line_number":132,"context_line":"        if max_simultaneous_jobs \u003e 0:"}],"source_content_type":"text/x-python","patch_set":15,"id":"1a26ad4f_3fd32b06","line":129,"in_reply_to":"1a26ad4f_79c4685e","updated":"2015-10-30 00:34:53.000000000","message":"Sure, will do.","commit_id":"3d62d7fd500491bb2ee0a936ac8ca6e676e43f09"},{"author":{"_account_id":5164,"name":"jsimms","email":"justin.simms@gmail.com","username":"jsimms"},"change_message_id":"94c22fca857333126e4da6421fe77e137479074c","unresolved":false,"context_lines":[{"line_number":247,"context_line":"                self._log.warn(\"Job dispatching interrupted: %s\", job)"},{"line_number":248,"context_line":"        except Exception:"},{"line_number":249,"context_line":"            self._log.warn(\"Job dispatching failed: %s\", job, exc_info\u003dTrue)"},{"line_number":250,"context_line":"        finally:"},{"line_number":251,"context_line":"            self._try_finish_job(job, consume)"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    def _run_until_dead(self, executor, max_dispatches\u003dNone):"}],"source_content_type":"text/x-python","patch_set":15,"id":"1a26ad4f_398cb034","line":250,"updated":"2015-10-29 17:03:24.000000000","message":"Would a log statement be useful here for tracing job transitions?","commit_id":"3d62d7fd500491bb2ee0a936ac8ca6e676e43f09"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"0db4734e6c35867fd8072104dae7881d93672508","unresolved":false,"context_lines":[{"line_number":247,"context_line":"                self._log.warn(\"Job dispatching interrupted: %s\", job)"},{"line_number":248,"context_line":"        except Exception:"},{"line_number":249,"context_line":"            self._log.warn(\"Job dispatching failed: %s\", job, exc_info\u003dTrue)"},{"line_number":250,"context_line":"        finally:"},{"line_number":251,"context_line":"            self._try_finish_job(job, consume)"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    def _run_until_dead(self, executor, max_dispatches\u003dNone):"}],"source_content_type":"text/x-python","patch_set":15,"id":"1a26ad4f_7fe8c3b4","line":250,"in_reply_to":"1a26ad4f_398cb034","updated":"2015-10-30 00:34:53.000000000","message":"Sure.","commit_id":"3d62d7fd500491bb2ee0a936ac8ca6e676e43f09"},{"author":{"_account_id":5164,"name":"jsimms","email":"justin.simms@gmail.com","username":"jsimms"},"change_message_id":"94c22fca857333126e4da6421fe77e137479074c","unresolved":false,"context_lines":[{"line_number":283,"context_line":"                            self._log.debug(\"Job already claimed or\""},{"line_number":284,"context_line":"                                            \" consumed: %s\", job)"},{"line_number":285,"context_line":"                        else:"},{"line_number":286,"context_line":"                            try:"},{"line_number":287,"context_line":"                                fut \u003d executor.submit(self._dispatch_job, job)"},{"line_number":288,"context_line":"                            except RuntimeError:"},{"line_number":289,"context_line":"                                with excutils.save_and_reraise_exception():"}],"source_content_type":"text/x-python","patch_set":15,"id":"1a26ad4f_b978c02c","line":286,"updated":"2015-10-29 17:03:24.000000000","message":"Would a logging statement be useful here, since the claimed job is about to be dispatched?","commit_id":"3d62d7fd500491bb2ee0a936ac8ca6e676e43f09"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"0db4734e6c35867fd8072104dae7881d93672508","unresolved":false,"context_lines":[{"line_number":283,"context_line":"                            self._log.debug(\"Job already claimed or\""},{"line_number":284,"context_line":"                                            \" consumed: %s\", job)"},{"line_number":285,"context_line":"                        else:"},{"line_number":286,"context_line":"                            try:"},{"line_number":287,"context_line":"                                fut \u003d executor.submit(self._dispatch_job, job)"},{"line_number":288,"context_line":"                            except RuntimeError:"},{"line_number":289,"context_line":"                                with excutils.save_and_reraise_exception():"}],"source_content_type":"text/x-python","patch_set":15,"id":"1a26ad4f_9feb5fb0","line":286,"in_reply_to":"1a26ad4f_b978c02c","updated":"2015-10-30 00:34:53.000000000","message":"Sure.","commit_id":"3d62d7fd500491bb2ee0a936ac8ca6e676e43f09"},{"author":{"_account_id":9648,"name":"Greg Hill","email":"greg.hill@rackspace.com","username":"jimbobhickville"},"change_message_id":"49629bdcba822f601c7660ac5c48f96456c350ae","unresolved":false,"context_lines":[{"line_number":276,"context_line":"            max_dispatches \u003d -1"},{"line_number":277,"context_line":"        dispatch_gen \u003d iter_utils.iter_forever(max_dispatches)"},{"line_number":278,"context_line":"        is_stopped \u003d self._waiter.is_stopped"},{"line_number":279,"context_line":"        not_is_stopped \u003d lambda *args: not is_stopped()"},{"line_number":280,"context_line":"        try:"},{"line_number":281,"context_line":"            # Don\u0027t even do any work in the first place..."},{"line_number":282,"context_line":"            if max_dispatches \u003d\u003d 0:"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba8a016a_35c7a897","line":279,"updated":"2015-11-18 15:17:40.000000000","message":"Super nitpicky, but maybe \u0027is_not_stopped\u0027.  Reads a lot better inline that way.","commit_id":"d72893206033585c822c907693544ff8c34ed643"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"50768f03ca15e7adfce4fa63359df634a282fb57","unresolved":false,"context_lines":[{"line_number":276,"context_line":"            max_dispatches \u003d -1"},{"line_number":277,"context_line":"        dispatch_gen \u003d iter_utils.iter_forever(max_dispatches)"},{"line_number":278,"context_line":"        is_stopped \u003d self._waiter.is_stopped"},{"line_number":279,"context_line":"        not_is_stopped \u003d lambda *args: not is_stopped()"},{"line_number":280,"context_line":"        try:"},{"line_number":281,"context_line":"            # Don\u0027t even do any work in the first place..."},{"line_number":282,"context_line":"            if max_dispatches \u003d\u003d 0:"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba8a016a_30d7a44c","line":279,"in_reply_to":"ba8a016a_35c7a897","updated":"2015-11-18 20:44:41.000000000","message":"works for me.","commit_id":"d72893206033585c822c907693544ff8c34ed643"},{"author":{"_account_id":9648,"name":"Greg Hill","email":"greg.hill@rackspace.com","username":"jimbobhickville"},"change_message_id":"49629bdcba822f601c7660ac5c48f96456c350ae","unresolved":false,"context_lines":[{"line_number":290,"context_line":"                        # waiting to dispatch a job into the executor (due"},{"line_number":291,"context_line":"                        # to being limited by the concurrent amount of"},{"line_number":292,"context_line":"                        # dispatches); so give up and finish up..."},{"line_number":293,"context_line":"                        break"},{"line_number":294,"context_line":"                    delayed_release \u003d False"},{"line_number":295,"context_line":"                    try:"},{"line_number":296,"context_line":"                        self._log.debug(\"Trying to claim job: %s\", job)"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba8a016a_d5a89c8f","line":293,"updated":"2015-11-18 15:17:40.000000000","message":"So, when we exhaust our max_dispatches, we stop this loop, but we then just restart the loop because not_is_stopped is still false.  Should not_is_stopped also check for the limiter being exhausted?  Or is there some piece of the puzzle I\u0027m not seeing?","commit_id":"d72893206033585c822c907693544ff8c34ed643"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"50768f03ca15e7adfce4fa63359df634a282fb57","unresolved":false,"context_lines":[{"line_number":290,"context_line":"                        # waiting to dispatch a job into the executor (due"},{"line_number":291,"context_line":"                        # to being limited by the concurrent amount of"},{"line_number":292,"context_line":"                        # dispatches); so give up and finish up..."},{"line_number":293,"context_line":"                        break"},{"line_number":294,"context_line":"                    delayed_release \u003d False"},{"line_number":295,"context_line":"                    try:"},{"line_number":296,"context_line":"                        self._log.debug(\"Trying to claim job: %s\", job)"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba8a016a_306564b4","line":293,"in_reply_to":"ba8a016a_d5a89c8f","updated":"2015-11-18 20:44:41.000000000","message":"not really, when we exhaust our max_dispatches the acquire() blocks, the only way it returns false if the semaphore was intertuppted (stopped), which happens via the \u0027stop(self, timeout\u003dNone)\u0027 in this class, and that method before interuptting the semaphore stops the self._waiter, so \u0027itertools.takewhile\u0027 will stop that and while not_is_stopped will also stop...\n\nthats the reason why this uses a semaphore (that had to be changed to be interruptible, since the built-in python threading one isn\u0027t, and it will block up forever...)","commit_id":"d72893206033585c822c907693544ff8c34ed643"}],"taskflow/utils/threading_utils.py":[{"author":{"_account_id":5164,"name":"jsimms","email":"justin.simms@gmail.com","username":"jsimms"},"change_message_id":"5277deed9fd4ed1e88cf82e6e0d65c2cc8a9ea36","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        self.release()"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    def acquire(self):"},{"line_number":40,"context_line":"        \"\"\"Does nothing.\"\"\""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    def release(self):"},{"line_number":43,"context_line":"        \"\"\"Does nothing.\"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"1a26ad4f_58ee8223","line":40,"updated":"2015-10-28 03:00:10.000000000","message":"Why did you decide not to use a pass statement here?","commit_id":"3a8dac62ad854aa35343798e69beb96b1acfd81e"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"e2c7c6c715ddb052bb0f486b2c926cbefce3ed84","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        self.release()"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    def acquire(self):"},{"line_number":40,"context_line":"        \"\"\"Does nothing.\"\"\""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    def release(self):"},{"line_number":43,"context_line":"        \"\"\"Does nothing.\"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"1a26ad4f_9a040d16","line":40,"in_reply_to":"1a26ad4f_58ee8223","updated":"2015-10-28 05:16:26.000000000","message":"The string here makes that not needed :)\n\nThis is a weird edge case of python where pass isn\u0027t needed since python afaik is thinking the string here is used (although it isn\u0027t).","commit_id":"3a8dac62ad854aa35343798e69beb96b1acfd81e"},{"author":{"_account_id":5164,"name":"jsimms","email":"justin.simms@gmail.com","username":"jsimms"},"change_message_id":"5277deed9fd4ed1e88cf82e6e0d65c2cc8a9ea36","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        \"\"\"Does nothing.\"\"\""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    def release(self):"},{"line_number":43,"context_line":"        \"\"\"Does nothing.\"\"\""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"def make_semaphore_factory(sema_cls):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a26ad4f_f8f2ee05","line":43,"updated":"2015-10-28 03:00:10.000000000","message":"Same comment as above.","commit_id":"3a8dac62ad854aa35343798e69beb96b1acfd81e"},{"author":{"_account_id":5164,"name":"jsimms","email":"justin.simms@gmail.com","username":"jsimms"},"change_message_id":"b95d8cdd8303357a27b7e653546165beff4a30b2","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    __slots__ \u003d [\u0027_value\u0027, \u0027_stopped\u0027, \u0027_cond\u0027]"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    def __init__(self, value\u003d1, cond_factory\u003dthreading.Condition):"},{"line_number":37,"context_line":"        if value \u003c 0:"},{"line_number":38,"context_line":"            raise ValueError(\"Semaphore initial value must be greater or\""},{"line_number":39,"context_line":"                             \" equal to zero and not \u0027%s\u0027\" % (value))"},{"line_number":40,"context_line":"        self._value \u003d value"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a26ad4f_9d52c712","line":37,"updated":"2015-10-28 06:14:50.000000000","message":"Ok. I am going to ask a dumb question: If the default value for the semaphore is 1 when none is provided by the caller, why would you allow for 0 to be a valid value, since you block until you have a value greater than 0?","commit_id":"d2a065e51bacb805a92837291feeaacf43147969"},{"author":{"_account_id":5164,"name":"jsimms","email":"justin.simms@gmail.com","username":"jsimms"},"change_message_id":"2fb17a4ab2a661fa8635193f8019765d0d795791","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    __slots__ \u003d [\u0027_value\u0027, \u0027_stopped\u0027, \u0027_cond\u0027]"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    def __init__(self, value\u003d1, cond_factory\u003dthreading.Condition):"},{"line_number":37,"context_line":"        if value \u003c 0:"},{"line_number":38,"context_line":"            raise ValueError(\"Semaphore initial value must be greater or\""},{"line_number":39,"context_line":"                             \" equal to zero and not \u0027%s\u0027\" % (value))"},{"line_number":40,"context_line":"        self._value \u003d value"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a26ad4f_dd691f8d","line":37,"in_reply_to":"1a26ad4f_3d6e13ca","updated":"2015-10-28 06:25:08.000000000","message":"Hmm. I see your point, but consider this: If you have a single thread and the semaphore value is explicitly set to 0, then I do not think that thread would ever get a chance to do any work, since the acquire() would block indefinitely , waiting for the value to go to 1, right? I believe this also applies for more than one thread.","commit_id":"d2a065e51bacb805a92837291feeaacf43147969"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"9e4180d07ef7398779d8c9697685c7e5c7eeb32a","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    __slots__ \u003d [\u0027_value\u0027, \u0027_stopped\u0027, \u0027_cond\u0027]"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    def __init__(self, value\u003d1, cond_factory\u003dthreading.Condition):"},{"line_number":37,"context_line":"        if value \u003c 0:"},{"line_number":38,"context_line":"            raise ValueError(\"Semaphore initial value must be greater or\""},{"line_number":39,"context_line":"                             \" equal to zero and not \u0027%s\u0027\" % (value))"},{"line_number":40,"context_line":"        self._value \u003d value"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a26ad4f_3d6e13ca","line":37,"in_reply_to":"1a26ad4f_9d52c712","updated":"2015-10-28 06:17:29.000000000","message":"Soooo,\n\nMainly because:\n\nhttps://docs.python.org/2/library/threading.html#semaphore-objects\n\n    \u0027\u0027\u0027\n    The optional argument gives the initial value for the internal counter; it\n    defaults to 1. If the value given is less than 0, ValueError is raised.\n    \u0027\u0027\u0027\n\nJust mirroring that behavior here; why they let zero be a valid value idk, ha.","commit_id":"d2a065e51bacb805a92837291feeaacf43147969"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"fe0fd5925406d4655aca7e06f0187f5435527a51","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    __slots__ \u003d [\u0027_value\u0027, \u0027_stopped\u0027, \u0027_cond\u0027]"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    def __init__(self, value\u003d1, cond_factory\u003dthreading.Condition):"},{"line_number":37,"context_line":"        if value \u003c 0:"},{"line_number":38,"context_line":"            raise ValueError(\"Semaphore initial value must be greater or\""},{"line_number":39,"context_line":"                             \" equal to zero and not \u0027%s\u0027\" % (value))"},{"line_number":40,"context_line":"        self._value \u003d value"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a26ad4f_dd9cbf6b","line":37,"in_reply_to":"1a26ad4f_dd691f8d","updated":"2015-10-28 06:28:40.000000000","message":"See make_interruptible_semaphore_factory which is why its used (and the associated dummy thread); agreed that allowing zero is weird, but this is just an internal utility, and matches the python semaphore weirdness, usage currently all goes through make_interruptible_semaphore_factory so that seems ok (since that handles the less than or equal to zero case in a manner that works for taskflow).","commit_id":"d2a065e51bacb805a92837291feeaacf43147969"}]}