> On Dec. 17, 2013, 6:14 p.m., Brian Wickman wrote: > > src/main/python/twitter/aurora/client/cli/jobs.py, line 130 > > <https://reviews.apache.org/r/16306/diff/1/?file=398463#file398463line130> > > > > not sure if a schema was defined in the designdoc, but any reason not > > to just use TJSONProtocol.TSimpleJSONProtocol and convert directly from > > thrift to json here?
I ended up being a bit more baroque here; even TSimpleJSONProtocol doesn't do prettyprinting; it generates compact JSON with no whitespace. So I'm grabbing the output from thrift, scarfing it up through python's json library, and then prettyprinting it. > On Dec. 17, 2013, 6:14 p.m., Brian Wickman wrote: > > src/main/python/twitter/aurora/client/cli/jobs.py, lines 150-155 > > <https://reviews.apache.org/r/16306/diff/1/?file=398463#file398463line150> > > > > yikes -- a leaky abstraction! we should re-file AURORA-3280 externally. This looked sufficiently neutered to me that I assumed it was a deliberate stub. > On Dec. 17, 2013, 6:14 p.m., Brian Wickman wrote: > > src/main/python/twitter/aurora/client/cli/jobs.py, line 217 > > <https://reviews.apache.org/r/16306/diff/1/?file=398463#file398463line217> > > > > meta-question: does it make sense to put a logger on the context > > instead of using print? (even if the logger _is_ print?) Excellent idea! - Mark ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16306/#review30575 ----------------------------------------------------------- On Dec. 16, 2013, 5:19 p.m., Mark Chu-Carroll wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16306/ > ----------------------------------------------------------- > > (Updated Dec. 16, 2013, 5:19 p.m.) > > > Review request for Aurora, Jonathan Boulle and Brian Wickman. > > > Repository: aurora > > > Description > ------- > > Added an implementation of the "status" verb for jobs. This is the first > command to use the new job wildcards, so there's a bunch of infrastructure > added to the Context to support it. > > This also includes unit tests of the new command. > > > Diffs > ----- > > src/main/python/twitter/aurora/client/api/__init__.py > 60f4011dda0db17ed57d3499637270d466c1b0b8 > src/main/python/twitter/aurora/client/cli/BUILD > aaeede4ac0b1239c1cb9354d15365658e1650ec8 > src/main/python/twitter/aurora/client/cli/__init__.py > 4b771d61464cfa2256d48f0a71ddd5829dd27266 > src/main/python/twitter/aurora/client/cli/context.py > 2ae92ecce8ec3ee2bf66b727e19bc90d1c171eb9 > src/main/python/twitter/aurora/client/cli/jobs.py > 58a723fbc5591565fd51311e0dcfa9567282dbb3 > src/test/python/twitter/aurora/client/cli/BUILD > 89184b649f8c7807a6f599d6f0026902eed11a6c > src/test/python/twitter/aurora/client/cli/test_create.py > da2828f73d32e3d3305fddd4224014a9b847ad2a > src/test/python/twitter/aurora/client/cli/test_kill.py > 8f8095cd2d1dac9d6d2a9891db16cce1d4227eaf > src/test/python/twitter/aurora/client/cli/test_status.py PRE-CREATION > src/test/python/twitter/aurora/client/cli/util.py > 46883ecd696b0f6b691831b65bb5e71367cd652a > > Diff: https://reviews.apache.org/r/16306/diff/ > > > Testing > ------- > > > Thanks, > > Mark Chu-Carroll > >
