> On Aug. 14, 2014, 5:09 p.m., Mark Chu-Carroll wrote: > > src/main/python/apache/aurora/client/api/updater_util.py, line 67 > > <https://reviews.apache.org/r/24702/diff/1/?file=660625#file660625line67> > > > > This is a bit magical for me. It's relying on subtleties of methods > > that a typical python programmer will need to look up, which means that > > it's going to be really easy for it to be wrong in a hard-to-recognize way. > > > > Can you move it up, so that it's a classmethod, and then add a couple > > of tests? Tests will both check that it works, and provide demonstrations > > for readers to see how it works. (I know you've already got some tests of > > this, but they're indirect, htrough the surrounding method; for clarity, it > > would be better to be able to test get_ranges directly.)
Moved into a classmethod, added comments and moved unit tests into a separate file. > On Aug. 14, 2014, 5:09 p.m., Mark Chu-Carroll wrote: > > src/main/python/apache/aurora/client/api/__init__.py, line 179 > > <https://reviews.apache.org/r/24702/diff/1/?file=660624#file660624line179> > > > > Can you add a more specific error type - maybe InvalidConfigError? That > > way, in the client, I can generate better error messages. Sure, done. - Maxim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24702/#review50593 ----------------------------------------------------------- On Aug. 14, 2014, 4:08 p.m., Maxim Khutornenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24702/ > ----------------------------------------------------------- > > (Updated Aug. 14, 2014, 4:08 p.m.) > > > Review request for Aurora, Mark Chu-Carroll and Bill Farner. > > > Bugs: AURORA-615 > https://issues.apache.org/jira/browse/AURORA-615 > > > Repository: aurora > > > Description > ------- > > Implementing client job lock and start update APIs. > > > Diffs > ----- > > src/main/python/apache/aurora/client/api/__init__.py > 371137e469fd65935f4048eaafdcf9b702e8c947 > src/main/python/apache/aurora/client/api/updater_util.py > cbb93e4d64a334dba9d091b68cf6d3594930fa31 > src/test/python/apache/aurora/client/api/BUILD > db5c22375378566f58ade5d6a0b7fcce75ad8394 > src/test/python/apache/aurora/client/api/test_api.py PRE-CREATION > > Diff: https://reviews.apache.org/r/24702/diff/ > > > Testing > ------- > > ./pants src/test/python/apache/aurora/client/api:api > > > Thanks, > > Maxim Khutornenko > >
