> On Sept. 2, 2014, 7:35 p.m., Mark Chu-Carroll wrote: > > src/main/python/apache/aurora/client/api/__init__.py, line 176 > > <https://reviews.apache.org/r/25204/diff/1/?file=672539#file672539line176> > > > > Nit - but why are you changing the parameter comment syntax? We don't > > use the double-dash anywhere else in the client.
This is the style everything under client/api seems to follow. Just making it consistent. > On Sept. 2, 2014, 7:35 p.m., Mark Chu-Carroll wrote: > > src/main/python/apache/aurora/client/api/__init__.py, line 232 > > <https://reviews.apache.org/r/25204/diff/1/?file=672539#file672539line232> > > > > I think this would be clearer inlined. Right now, it's pretty much an > > alternate name for the JobUpdateQuery constructor, but with different > > parameter names. It makes the code harder to follow, not easier. The reason for this is mostly consistency (see build_query above) but I personally prefer this approach as it decouples query building from the thrift API call. > On Sept. 2, 2014, 7:35 p.m., Mark Chu-Carroll wrote: > > src/main/python/apache/aurora/client/api/__init__.py, line 252 > > <https://reviews.apache.org/r/25204/diff/1/?file=672539#file672539line252> > > > > If we're going to the trouble of abstracting away the actual scheduler > > interface, I think we shouldn't be returning the raw Response datatype. > > This should return the updates, not the structure wrapping the updates in > > two layers of indirection. > > > > These kinds of methods should check the result code, raise an exception > > if the API call failed, and then return a meaningful, simple result. > > > > (Same comment applies to get_job_update_details) Make it 3 for consistency :) Nothing in that file attempts to return a result. Event the updater tries to fake a Response object in order to comply with general api pattern. - Maxim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25204/#review52064 ----------------------------------------------------------- On Aug. 29, 2014, 10:28 p.m., Maxim Khutornenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25204/ > ----------------------------------------------------------- > > (Updated Aug. 29, 2014, 10:28 p.m.) > > > Review request for Aurora and Mark Chu-Carroll. > > > Bugs: AURORA-615 > https://issues.apache.org/jira/browse/AURORA-615 > > > Repository: aurora > > > Description > ------- > > Adding "get" job update client APIs. > > > Diffs > ----- > > src/main/python/apache/aurora/client/api/__init__.py > 90462bf5920786ea1316c75a99c8382cf8c803a1 > src/test/python/apache/aurora/client/api/test_api.py > b47b6db31842fffba797c7f616b5f4deb8d04a86 > > Diff: https://reviews.apache.org/r/25204/diff/ > > > Testing > ------- > > ./pants src/test/python/apache/aurora/client/api:api -s > > > Thanks, > > Maxim Khutornenko > >
