> On Oct. 24, 2016, 4:04 p.m., Joshua Cohen wrote:
> > Thanks once again for the patch!
> > 
> > If I'm missing something and the changes on the non-json rendering side are 
> > necessary, can you also ensure that the output there is identical 
> > before/after your patch (i.e. run `aurora job inspect` on master and this 
> > branch and diff the output)?

the only difference for non-json rendering is contraint. I use 2 spaces to 
indent the containts content. For example,

beofre:
```
"constraints":
"fetch package < hello world"
```
now:
```
"constraints":
  "fetch package < hello world"
```
If the indentation is not necessary for contraints, I could remove it.


> On Oct. 24, 2016, 4:04 p.m., Joshua Cohen wrote:
> > src/main/python/apache/aurora/client/cli/jobs.py, line 238
> > <https://reviews.apache.org/r/53114/diff/1/?file=1543714#file1543714line238>
> >
> >     Was the refactoring here to write to build up an array and then return 
> > the joined string necessary? It seems like this method is not called in the 
> > write json path? Can we just leave this as is and continue to directly use 
> > `context.print_out` to render (which has the slight benefit of handling 
> > indentation if the value is multi-line)?

It is the refactoring here of human readable configuration output, the reason 
for the method is only for readability, when the option is enabled, the output 
will be in json format, otherwise, it prints out as before by 
_reder_config_pretty_. Once we enable --write-json option, I don't think it is 
necessary to print out those information since json output provides the same 
information, so the method won't be called, only json format is printed. 

One more question here is where to put those _context.print_outs_, should I put 
in separated method _render_config_pretty_ or _execute_? Thanks Joshua.


- Jing


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53114/#review153696
-----------------------------------------------------------


On Oct. 23, 2016, 12:37 a.m., Jing Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53114/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2016, 12:37 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Brian Wickman, and Zameer Manji.
> 
> 
> Bugs: AURORA-1504
>     https://issues.apache.org/jira/browse/AURORA-1504
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> aurora job inspect should have a --write-json option
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/client/cli/jobs.py 
> 87fbf13f8e5955b30436f8d871b548275f009893 
>   src/test/python/apache/aurora/client/cli/test_inspect.py 
> fedc16b3d4e9fb7d6f5f0dc34ad7a1837e34baea 
> 
> Diff: https://reviews.apache.org/r/53114/diff/
> 
> 
> Testing
> -------
> 
> * For configuration for a job as:
> ```
> pkg_path = '/vagrant/hello.py'
> 
> install = Process(
>     name='fetch package',
>     cmdline='cp {} . && chmod u+x hello.py'.format(pkg_path))
> 
> runner = Process(
>     name='hello jing',
>     ephemeral = True,
>     cmdline='python -u hello.py')
> 
> hello_task = SequentialTask(
>     name='hello task',
>     processes=[install, runner],
>     resources=Resources(cpu=1, ram=1*MB, disk=8*MB))
> 
> jobs = [
>     Service(cluster='devcluster',
>         role='www-data',
>         environment='devel',
>         name='hello_jing',
>         task=hello_task,
>         constraints = {
>               'host':'limit:2',
>               'rack':'limit:1'
>         })
> ]
> ```
> command 'aurora job inspect devcluster/www-data/devel/hello_jing 
> /vagrant/hello.aurora --write-json' returns:
> ```
> {
>   "job": {
>     "name": "hello_jing",
>     "role": "www-data",
>     "contact": "<class 'pystachio.composite.Empty'>",
>     "cluster": "devcluster",
>     "instances": "1",
>     "constraints": [
>       {
>         "host": "limit:2"
>       },
>       {
>         "rack": "limit:1"
>       }
>     ],
>     "service": true,
>     "production": false
>   },
>   "task": {
>     "name": "hello task",
>     "constraints": [
>       "fetch package < hello jing"
>     ],
>     "processes": [
>       {
>         "name": "fetch package",
>         "daemon": false,
>         "ephemeral": false,
>         "final": false,
>         "cmdline": [
>           "cp /vagrant/hello.py . && chmod u+x hello.py"
>         ]
>       },
>       {
>         "name": "hello jing",
>         "daemon": false,
>         "ephemeral": true,
>         "final": false,
>         "cmdline": [
>           "python -u hello.py"
>         ]
>       }
>     ]
>   }
> }
> 
> ```
> 
> * For configuration for a cron job as:
> ```
> jobs = [
>   Job(
>     cluster = 'devcluster',
>     role = 'www-data',
>     environment = 'devel',
>     name = 'cron_hello_jing',
>     cron_schedule = '*/10 * * * *',
>     cron_collision_policy='CANCEL_NEW',
>     task = Task(
>       name="cron_hello_jing",
>       processes=[Process(name="hello_jing",
>            cmdline="echo 'cron hello jing'")],
>       resources=Resources(cpu=1, ram=1*MB, disk=8*MB)
>     )
>   )
> ]
> ```
> command aurora job inspect devcluster/www-data/devel/cron_hello_jing 
> /vagrant/cron_hello_world.aurora --write-json returns:
> ```
> {
>   "job": {
>     "name": "cron_hello_jing",
>     "role": "www-data",
>     "contact": "<class 'pystachio.composite.Empty'>",
>     "cluster": "devcluster",
>     "instances": "1",
>     "cron": {
>       "schedule": "*/10 * * * *",
>       "policy": "CANCEL_NEW"
>     },
>     "service": false,
>     "production": false
>   },
>   "task": {
>     "name": "cron_hello_jing",
>     "processes": [
>       {
>         "name": "hello_jing",
>         "daemon": false,
>         "ephemeral": false,
>         "final": false,
>         "cmdline": [
>           "echo 'cron hello jing'"
>         ]
>       }
>     ]
>   }
> }
> ```
> 
> 
> Thanks,
> 
> Jing Chen
> 
>

Reply via email to