Alok Aggarwal wrote: > > On Tue, 19 Jan 2010, Keith Mitchell wrote: > >>> Also if you use "retry_timeout > 0", then you don't >>> execute the command even once if retry_timeout starts >>> out being '0'. >>> >>> So, "break" seemed like the most straightforward >>> and one that worked in my testing as well. >> >> What's really confusing is now we've got multiple conditionals and >> some of them are irrelevant, or seem irrelevant. For example, we >> check retry_timeout in two places, and status == 1 in two places. In >> particular, with your suggested "break", it seems like line 934 could >> simply be converted to "while True:" (which would follow a common >> paradigm for implementing "do-while" type syntax in Python, and seems >> like a good idea). An additional 'break' would be needed for "if >> status == 0", but I think it would make the entire code block much >> more legible. > > Does the updated webrev look better? > > http://cr.opensolaris.org/~aalok/13537-13766-13795-13979-13892/ > > It addresses the TAbort on a timeout as well.
Looks great! (Minor nit - line 933 isn't needed any more; no need for a new webrev if you remove it) Thanks, Keith > > Alok
