Re: Review Request 38385: Documenting dedicated job & quota relationship.

2015-09-21 Thread Maxim Khutornenko 


> On Sept. 16, 2015, 12:51 a.m., Bill Farner wrote:
> > docs/resource-isolation.md, line 150
> > 
> >
> > This doc is otherewise about machine-level resource isolation, so it 
> > seems like an odd match here.  It seems to align well with content in this 
> > page: docs/deploying-aurora-scheduler.md.

I actually found this doc less about isolation and more about resources in 
general. Renamed this page to better reflect what it currently is.


> On Sept. 16, 2015, 12:51 a.m., Bill Farner wrote:
> > docs/deploying-aurora-scheduler.md, line 212
> > 
> >
> > I think it's fine to omit this and let the 'Resource Quota' section 
> > stand on its own.  Without a pointer to context, this is difficult to piece 
> > together.  Perhaps this:
> > 
> > > See the section about resource quotas [link] to learn how quotas 
> > apply to dedicated jobs.

Done.


> On Sept. 16, 2015, 12:51 a.m., Bill Farner wrote:
> > docs/configuration-reference.md, line 327
> > 
> >
> > Not sure why we had the 'warning' in the first place, but i think it 
> > should be removed.  There isn't really any harm that warrants that tone.  
> > Second - the note about preemption would be _great_ in a separate 
> > mini-section near 'Resource Quota'.  With that, you can trim this down 
> > quite a bit:
> > 
> > > Indicated whether this is a production job that will be preferred for 
> > preemption [link].
> > 
> > Then the new preemption section can draw the connection to quota.

Added a new section and trimmed this quite a bit.


> On Sept. 16, 2015, 12:51 a.m., Bill Farner wrote:
> > docs/client-commands.md, line 335
> > 
> >
> > Please linkify `dedicated`, that would go a long way to point towards 
> > context.  A quick skim turnedt his up as a potential target: 
> > https://github.com/apache/aurora/blob/master/docs/deploying-aurora-scheduler.md#dedicated-attribute

Done.


- Maxim


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


On Sept. 16, 2015, 12:14 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38385/
> ---
> 
> (Updated Sept. 16, 2015, 12:14 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-1462
> https://issues.apache.org/jira/browse/AURORA-1462
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Documenting dedicated job & quota relationship.
> 
> 
> Diffs
> -
> 
>   docs/client-commands.md f91e5df0c19104a961aa9955478bf2d37a2aa7b6 
>   docs/configuration-reference.md ad2701cadd38bb2fdbbe2acc477038986f8ec733 
>   docs/configuration-tutorial.md d6e7c352e16964e19a41340cb03a016620c17f7d 
>   docs/deploying-aurora-scheduler.md 8db0e615b6abe6865a889dbcfb24271655caaee6 
>   docs/resource-isolation.md 7e8d88d09093d85c07c84bd3d6476fc89ff21c3b 
> 
> Diff: https://reviews.apache.org/r/38385/diff/
> 
> 
> Testing
> ---
> 
> Private remote: 
> https://github.com/maxim111333/incubator-aurora/tree/quota_dedicated_docs/docs
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 38385: Documenting dedicated job & quota relationship.

2015-09-21 Thread Maxim Khutornenko 

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

(Updated Sept. 21, 2015, 9:20 p.m.)


Review request for Aurora and Bill Farner.


Changes
---

Bill's comments.


Bugs: AURORA-1462
https://issues.apache.org/jira/browse/AURORA-1462


Repository: aurora


Description
---

Documenting dedicated job & quota relationship.


Diffs (updated)
-

  docs/README.md fe115dca4931adbae545243dd9195481eeb035a0 
  docs/client-commands.md f91e5df0c19104a961aa9955478bf2d37a2aa7b6 
  docs/configuration-reference.md ad2701cadd38bb2fdbbe2acc477038986f8ec733 
  docs/configuration-tutorial.md d6e7c352e16964e19a41340cb03a016620c17f7d 
  docs/deploying-aurora-scheduler.md 8db0e615b6abe6865a889dbcfb24271655caaee6 
  docs/resource-isolation.md 7e8d88d09093d85c07c84bd3d6476fc89ff21c3b 

Diff: https://reviews.apache.org/r/38385/diff/


Testing
---

Private remote: 
https://github.com/maxim111333/incubator-aurora/tree/quota_dedicated_docs/docs


Thanks,

Maxim Khutornenko

Re: Review Request 38390: Adding oversubscription summary.

2015-09-21 Thread Maxim Khutornenko 


> On Sept. 15, 2015, 1:20 a.m., Bill Farner wrote:
> > docs/configuration-reference.md, lines 344-355
> > 
> >
> > Tiers seems like a significant enough topic to warrant its own page 
> > with some more context and better flow.  As it stands, it's really hard to 
> > understand what exactly a tier is and why i want or need to define them. 
> > 
> > Also, you might consider dropping the term 'best-effort' and stick to 
> > 'revocable' to avoid overloading the naming.
> 
> Maxim Khutornenko wrote:
> | Tiers seems like a significant enough topic to warrant its own page 
> with some more context and better flow.  As it stands, it's really hard to 
> understand what exactly a tier is and why i want or need to define them. 
> 
> Agree. I am hesitant to promote the `tier` concept though until we fully 
> conceptualize it in AURORA-1443. As it stands now, it only supports 
> `revocable` value and has zero meaning outside the revocable offer work. 
> 
> | Also, you might consider dropping the term 'best-effort' and stick to 
> 'revocable' to avoid overloading the naming.
> 
> Sure, works for me.
> 
> Bill Farner wrote:
> The intention is not to promote it (disclaimers are fine), but rather to 
> paint the picture and not leave the reader hanging.  Doesn't have to be a lot 
> of content, but some indication of how it fits in and the development status 
> would go a long way.

I am uncomfortable promoting a half-baked concept until the migration plan and 
implementation details are fully fleshed out. I feel that AURORA-1343 and 
AURORA-1443 linked above give enough additional info for curiously minded 
without restating the points already defined in the design doc (mentioned in 
the above tickets).


- Maxim


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


On Sept. 16, 2015, 12:38 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38390/
> ---
> 
> (Updated Sept. 16, 2015, 12:38 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-1441
> https://issues.apache.org/jira/browse/AURORA-1441
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding oversubscription summary.
> 
> 
> Diffs
> -
> 
>   docs/configuration-reference.md ad2701cadd38bb2fdbbe2acc477038986f8ec733 
>   docs/deploying-aurora-scheduler.md 8db0e615b6abe6865a889dbcfb24271655caaee6 
> 
> Diff: https://reviews.apache.org/r/38390/diff/
> 
> 
> Testing
> ---
> 
> Private remote: 
> https://github.com/maxim111333/incubator-aurora/blob/oversubscription_docs/docs/
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 38390: Adding oversubscription summary.

2015-09-21 Thread Zameer Manji 

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

Ship it!


LGTM modulo my one nit.


docs/configuration-reference.md (line 355)


What happens if a job with a tier of revocable is sent to a scheduler where 
it isn't configured? I think that should be added here.


- Zameer Manji


On Sept. 21, 2015, 2:41 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38390/
> ---
> 
> (Updated Sept. 21, 2015, 2:41 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1441
> https://issues.apache.org/jira/browse/AURORA-1441
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding oversubscription summary.
> 
> 
> Diffs
> -
> 
>   docs/configuration-reference.md ad2701cadd38bb2fdbbe2acc477038986f8ec733 
>   docs/deploying-aurora-scheduler.md 8db0e615b6abe6865a889dbcfb24271655caaee6 
> 
> Diff: https://reviews.apache.org/r/38390/diff/
> 
> 
> Testing
> ---
> 
> Private remote: 
> https://github.com/maxim111333/incubator-aurora/blob/oversubscription_docs/docs/
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 38385: Documenting dedicated job & quota relationship.

2015-09-21 Thread Zameer Manji 

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

Ship it!


LGTM modulo my nit below.


docs/resource-isolation.md (line 164)


higher or equal priority.


- Zameer Manji


On Sept. 21, 2015, 2:41 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38385/
> ---
> 
> (Updated Sept. 21, 2015, 2:41 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1462
> https://issues.apache.org/jira/browse/AURORA-1462
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Documenting dedicated job & quota relationship.
> 
> 
> Diffs
> -
> 
>   docs/README.md fe115dca4931adbae545243dd9195481eeb035a0 
>   docs/client-commands.md f91e5df0c19104a961aa9955478bf2d37a2aa7b6 
>   docs/configuration-reference.md ad2701cadd38bb2fdbbe2acc477038986f8ec733 
>   docs/configuration-tutorial.md d6e7c352e16964e19a41340cb03a016620c17f7d 
>   docs/deploying-aurora-scheduler.md 8db0e615b6abe6865a889dbcfb24271655caaee6 
>   docs/resource-isolation.md 7e8d88d09093d85c07c84bd3d6476fc89ff21c3b 
> 
> Diff: https://reviews.apache.org/r/38385/diff/
> 
> 
> Testing
> ---
> 
> Private remote: 
> https://github.com/maxim111333/incubator-aurora/tree/quota_dedicated_docs/docs
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 38385: Documenting dedicated job & quota relationship.

2015-09-21 Thread Aurora ReviewBot 

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

Ship it!


Master (1ff336e) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Sept. 21, 2015, 9:41 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38385/
> ---
> 
> (Updated Sept. 21, 2015, 9:41 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1462
> https://issues.apache.org/jira/browse/AURORA-1462
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Documenting dedicated job & quota relationship.
> 
> 
> Diffs
> -
> 
>   docs/README.md fe115dca4931adbae545243dd9195481eeb035a0 
>   docs/client-commands.md f91e5df0c19104a961aa9955478bf2d37a2aa7b6 
>   docs/configuration-reference.md ad2701cadd38bb2fdbbe2acc477038986f8ec733 
>   docs/configuration-tutorial.md d6e7c352e16964e19a41340cb03a016620c17f7d 
>   docs/deploying-aurora-scheduler.md 8db0e615b6abe6865a889dbcfb24271655caaee6 
>   docs/resource-isolation.md 7e8d88d09093d85c07c84bd3d6476fc89ff21c3b 
> 
> Diff: https://reviews.apache.org/r/38385/diff/
> 
> 
> Testing
> ---
> 
> Private remote: 
> https://github.com/maxim111333/incubator-aurora/tree/quota_dedicated_docs/docs
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 38326: Adding ssh options into "aurora task" commands.

2015-09-21 Thread Maxim Khutornenko 


> On Sept. 16, 2015, 12:31 a.m., Bill Farner wrote:
> > src/main/python/apache/aurora/client/api/command_runner.py, line 94
> > 
> >
> > This patch uses mutable lists in several places, which i feel is a 
> > divergence from general prefernce for immutability.  I don't think it 
> > hinders readability at all to assign these lists once and not mutate them.

The options param is a list. This is the simplest solution to merge it right 
into the ssh_command without inserting it as a list.


- Maxim


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


On Sept. 11, 2015, 11:31 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38326/
> ---
> 
> (Updated Sept. 11, 2015, 11:31 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1491
> https://issues.apache.org/jira/browse/AURORA-1491
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding ssh options into "aurora task" commands.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/command_runner.py 
> c7238e274e53138187c2fe6fe5f14b7ae5f43ba2 
>   src/main/python/apache/aurora/client/cli/options.py 
> 41b13d6f0e1ed355ea8b958b875c20f065349465 
>   src/main/python/apache/aurora/client/cli/task.py 
> d1f2568ac0afdd95c65523fde41f0dd16670a7a8 
>   src/test/python/apache/aurora/client/cli/test_task.py 
> 3ad0b70a7d918055ffee34f593d108a28de6e9f9 
> 
> Diff: https://reviews.apache.org/r/38326/diff/
> 
> 
> Testing
> ---
> 
> In vagrant:
> ```
> vagrant@aurora:~$ aurora task run -v --ssh-user=vagrant --ssh-options='-v -k' 
> --executor-sandbox devcluster/www-data/prod/hello 'ls'
> DEBUG] Command=(['task', 'run', '-v', '--ssh-user=vagrant', '--ssh-options=-v 
> -k', '--executor-sandbox', 'devcluster/www-data/prod/hello', 'ls'])
> DEBUG] Using auth module: 
>  0x7f05d2b88250>
> DEBUG] Running command: ['ssh', '-n', '-q', '-v', '-k', 
> 'vagrant@192.168.33.7', u'cd 
> /var/lib/mesos/slaves/*/frameworks/*/executors/thermos-1442013544190-www-data-prod-hello-0-fe7fa2f1-e21b-4cc3-9107-0777da35537e/runs/latest;ls']
> 192.168.33.7:  OpenSSH_6.6.1, OpenSSL 1.0.1f 6 Jan 2014
> 192.168.33.7:  debug1: Reading configuration data /etc/ssh/ssh_config
> 192.168.33.7:  debug1: /etc/ssh/ssh_config line 19: Applying options for *
> 192.168.33.7:  debug1: /etc/ssh/ssh_config line 57: Applying options for *
> 192.168.33.7:  debug1: Connecting to 192.168.33.7 [192.168.33.7] port 22.
> 192.168.33.7:  debug1: Connection established.
> 192.168.33.7:  debug1: identity file /home/vagrant/.ssh/id_rsa type 1
> 192.168.33.7:  debug1: identity file /home/vagrant/.ssh/id_rsa-cert type -1
> 192.168.33.7:  debug1: identity file /home/vagrant/.ssh/id_dsa type -1
> 192.168.33.7:  debug1: identity file /home/vagrant/.ssh/id_dsa-cert type -1
> 192.168.33.7:  debug1: identity file /home/vagrant/.ssh/id_ecdsa type -1
> 192.168.33.7:  debug1: identity file /home/vagrant/.ssh/id_ecdsa-cert type -1
> 192.168.33.7:  debug1: identity file /home/vagrant/.ssh/id_ed25519 type -1
> 192.168.33.7:  debug1: identity file /home/vagrant/.ssh/id_ed25519-cert type 
> -1
> 192.168.33.7:  debug1: Enabling compatibility mode for protocol 2.0
> 192.168.33.7:  debug1: Local version string SSH-2.0-OpenSSH_6.6.1p1 
> Ubuntu-2ubuntu2
> 192.168.33.7:  debug1: Remote protocol version 2.0, remote software version 
> OpenSSH_6.6.1p1 Ubuntu-2ubuntu2
> 192.168.33.7:  debug1: match: OpenSSH_6.6.1p1 Ubuntu-2ubuntu2 pat 
> OpenSSH_6.6.1* compat 0x0400
> 192.168.33.7:  debug1: SSH2_MSG_KEXINIT sent
> 192.168.33.7:  debug1: SSH2_MSG_KEXINIT received
> 192.168.33.7:  debug1: kex: server->client aes128-ctr 
> hmac-md5-...@openssh.com none
> 192.168.33.7:  debug1: kex: client->server aes128-ctr 
> hmac-md5-...@openssh.com none
> 192.168.33.7:  debug1: sending SSH2_MSG_KEX_ECDH_INIT
> 192.168.33.7:  debug1: expecting SSH2_MSG_KEX_ECDH_REPLY
> 192.168.33.7:  debug1: Server host key: ECDSA 
> 46:15:09:58:38:39:76:02:68:e1:c2:43:1f:70:bf:6e
> 192.168.33.7:  Warning: Permanently added '192.168.33.7' (ECDSA) to the list 
> of known hosts.
> 192.168.33.7:  debug1: ssh_ecdsa_verify: signature correct
> 192.168.33.7:  debug1: SSH2_MSG_NEWKEYS sent
> 192.168.33.7:  debug1: expecting SSH2_MSG_NEWKEYS
> 192.168.33.7:  debug1: SSH2_MSG_NEWKEYS received
> 192.168.33.7:  debug1: Roaming not allowed by server
> 192.168.33.7:  debug1: SSH2_MSG_SERVICE_REQUEST sent
> 192.168.33.7:  debug1: SSH2_MSG_SERVICE_ACCEPT received
> 192.168.33.7:  debug1: Authentications that can continue: publickey,password
> 192.168.33.7:  debug1: Next authenticatio

Re: Review Request 38385: Documenting dedicated job & quota relationship.

2015-09-21 Thread Maxim Khutornenko 


> On Sept. 21, 2015, 10:12 p.m., Zameer Manji wrote:
> > docs/resource-isolation.md, line 168
> > 
> >
> > higher or equal priority.

Negative: 
https://github.com/apache/aurora/blob/0070a5fd18c6f219a7fe66f327209b8dc21ab67e/src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java#L203


- Maxim


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


On Sept. 21, 2015, 9:41 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38385/
> ---
> 
> (Updated Sept. 21, 2015, 9:41 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1462
> https://issues.apache.org/jira/browse/AURORA-1462
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Documenting dedicated job & quota relationship.
> 
> 
> Diffs
> -
> 
>   docs/README.md fe115dca4931adbae545243dd9195481eeb035a0 
>   docs/client-commands.md f91e5df0c19104a961aa9955478bf2d37a2aa7b6 
>   docs/configuration-reference.md ad2701cadd38bb2fdbbe2acc477038986f8ec733 
>   docs/configuration-tutorial.md d6e7c352e16964e19a41340cb03a016620c17f7d 
>   docs/deploying-aurora-scheduler.md 8db0e615b6abe6865a889dbcfb24271655caaee6 
>   docs/resource-isolation.md 7e8d88d09093d85c07c84bd3d6476fc89ff21c3b 
> 
> Diff: https://reviews.apache.org/r/38385/diff/
> 
> 
> Testing
> ---
> 
> Private remote: 
> https://github.com/maxim111333/incubator-aurora/tree/quota_dedicated_docs/docs
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 38390: Adding oversubscription summary.

2015-09-21 Thread Maxim Khutornenko 

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

(Updated Sept. 21, 2015, 10:36 p.m.)


Review request for Aurora, Bill Farner and Zameer Manji.


Changes
---

Zameer's comments.


Bugs: AURORA-1441
https://issues.apache.org/jira/browse/AURORA-1441


Repository: aurora


Description
---

Adding oversubscription summary.


Diffs (updated)
-

  docs/configuration-reference.md ad2701cadd38bb2fdbbe2acc477038986f8ec733 
  docs/deploying-aurora-scheduler.md 8db0e615b6abe6865a889dbcfb24271655caaee6 

Diff: https://reviews.apache.org/r/38390/diff/


Testing
---

Private remote: 
https://github.com/maxim111333/incubator-aurora/blob/oversubscription_docs/docs/


Thanks,

Maxim Khutornenko

Re: Review Request 38390: Adding oversubscription summary.

2015-09-21 Thread Maxim Khutornenko 


> On Sept. 21, 2015, 9:48 p.m., Zameer Manji wrote:
> > docs/configuration-reference.md, line 355
> > 
> >
> > What happens if a job with a tier of revocable is sent to a scheduler 
> > where it isn't configured? I think that should be added here.

Added a clarification.


- Maxim


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


On Sept. 21, 2015, 9:41 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38390/
> ---
> 
> (Updated Sept. 21, 2015, 9:41 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1441
> https://issues.apache.org/jira/browse/AURORA-1441
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding oversubscription summary.
> 
> 
> Diffs
> -
> 
>   docs/configuration-reference.md ad2701cadd38bb2fdbbe2acc477038986f8ec733 
>   docs/deploying-aurora-scheduler.md 8db0e615b6abe6865a889dbcfb24271655caaee6 
> 
> Diff: https://reviews.apache.org/r/38390/diff/
> 
> 
> Testing
> ---
> 
> Private remote: 
> https://github.com/maxim111333/incubator-aurora/blob/oversubscription_docs/docs/
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-09-21 Thread Renan DelValle 

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

(Updated Sept. 21, 2015, 11:42 p.m.)


Review request for Aurora and Bill Farner.


Changes
---

Switched JSON parser from GSON to Jackson. 

Changed ExcutorSettings to contain CommandInfo.Builder and eliminated other 
redudant fields as a result.

Changed executors-config schema to be a serialized version of 
CommandInfo.Builder, allowing us to configure CommandInfo from the file as per 
the mesos protobuf protocol.

This patch failes the end to end test, I haven't been able to pinpoint the 
reason why the serverset fails to announce the job but my hope is that more 
eyes on the code can help me fix this faster.


Repository: aurora


Description (updated)
---

This is the first stage in a series of patches to create support for custom 
executors. In an effort to expedite the review process, I have decided to break 
down my patch into multiple pieces that when/if commited won't break the trunk.

This patch includes the ability to load configuration from a JSON file. A JSON 
example file is included in examples/vagrant/executors-config.json

Command line arguments have been eliminated and moved over to the JSON file. 
Jackson is used to deserialize a CommandInfo.Builder along with two other 
fields. Volumes are now based on the Protobuf version and not the Aurora Thrift 
version.


Diffs (updated)
-

  .gitignore 86840972c53ea52a793968d3d00df6763a7d6ffb 
  3rdparty/python/requirements.txt 44217469a9583ec50233f34d54a32c105e6bab2c 
  CHANGELOG 3b6990fb4dc3cfb8c730c81063413e505c803471 
  NEWS a17f0e7c08fd30a0b2db6814a1c755111307228b 
  README.md ef9c2f0ecf44b03e2025fe9cab62a3aa5ae30a77 
  api/src/main/thrift/org/apache/aurora/gen/BUILD 
fe3f83b6a7680985dce01efe2d54ccc4b0c2c482 
  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
d740a90e7732f42b43a79f8cf0afe705c061539c 
  api/src/main/thrift/org/apache/aurora/gen/internal_rpc.thrift 
a2c230fa9b5f648c4674042411cbe46fb8bb4faa 
  api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
b7c4665d51f0efeebabd58c978707397b45d275d 
  api/src/main/thrift/org/apache/thermos/BUILD 
d0d789a6ee3971e3070f9397d53929563a77f7ea 
  build-support/python/isort 44f9659948703c75372cd70643d5631acb116c2e 
  build-support/python/isort-check 646cbf0bae4ccf8eac044817138ed1a7a59b261b 
  build-support/python/make-mesos-native-egg 
fce53e42dde72b893f291db9d78af1dc47fafe2f 
  build-support/python/make-pycharm-virtualenv 
05a16d0d421982bcfb2e649be7b83a17d813efb1 
  build-support/release/make-python-sdists 
9608f68e16243da01434ce2fc7d61bb7c7efd712 
  build.gradle 700e1ade1470b99e9be390db150cf73aa06f17bc 
  buildSrc/gradle.properties 3231a7add9e0503f0ee6779ae3c4000143b174ab 
  buildSrc/src/main/groovy/org/apache/aurora/build/CoverageReportCheck.groovy 
b996d4597492a76db0f2571db4e6c1ae9b4bcf33 
  commons-args/src/main/java/org/apache/aurora/common/args/Arg.java 
PRE-CREATION 
  commons-args/src/main/java/org/apache/aurora/common/args/ArgParser.java 
PRE-CREATION 
  commons-args/src/main/java/org/apache/aurora/common/args/CmdLine.java 
PRE-CREATION 
  commons-args/src/main/java/org/apache/aurora/common/args/NoParser.java 
PRE-CREATION 
  commons-args/src/main/java/org/apache/aurora/common/args/Parser.java 
PRE-CREATION 
  commons-args/src/main/java/org/apache/aurora/common/args/ParserOracle.java 
PRE-CREATION 
  commons-args/src/main/java/org/apache/aurora/common/args/Positional.java 
PRE-CREATION 
  commons-args/src/main/java/org/apache/aurora/common/args/Verifier.java 
PRE-CREATION 
  commons-args/src/main/java/org/apache/aurora/common/args/VerifierFor.java 
PRE-CREATION 
  
commons-args/src/main/java/org/apache/aurora/common/args/apt/CmdLineProcessor.java
 PRE-CREATION 
  
commons-args/src/main/java/org/apache/aurora/common/args/apt/Configuration.java 
PRE-CREATION 
  
commons-args/src/main/resources/META-INF/services/javax.annotation.processing.Processor
 PRE-CREATION 
  commons/src/main/java/org/apache/aurora/common/application/Lifecycle.java 
PRE-CREATION 
  
commons/src/main/java/org/apache/aurora/common/application/ShutdownRegistry.java
 PRE-CREATION 
  commons/src/main/java/org/apache/aurora/common/application/ShutdownStage.java 
PRE-CREATION 
  commons/src/main/java/org/apache/aurora/common/args/ArgFilters.java 
PRE-CREATION 
  commons/src/main/java/org/apache/aurora/common/args/ArgScanner.java 
PRE-CREATION 
  commons/src/main/java/org/apache/aurora/common/args/Args.java PRE-CREATION 
  commons/src/main/java/org/apache/aurora/common/args/ArgumentInfo.java 
PRE-CREATION 
  commons/src/main/java/org/apache/aurora/common/args/OptionInfo.java 
PRE-CREATION 
  commons/src/main/java/org/apache/aurora/common/args/Parsers.java PRE-CREATION 
  commons/src/main/java/org/apache/aurora/common/args/PositionalInfo.java 
PRE-CREATION 
  commons/src/main/java/org/apach

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-09-21 Thread Renan DelValle 


> On Sept. 2, 2015, 4:45 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/configuration/ExecutorSettingsLoader.java,
> >  line 87
> > 
> >
> > This code will be easier to maintain long-term with 2 goals in mind:
> > - keep json-related code within this class (no annotations on 
> > ExecutorSettings)
> > - avoid hand-rolling deserializers
> > 
> > This sounds like more work, but it's not that bad.  The major 
> > difference is that you'll create a class in this file that defines the JSON 
> > structure, and does so in a way that the json deserializer can work with.
> > 
> > Maxim just did something very similar, which you can crib from:
> > 
> > https://github.com/apache/aurora/blob/89da936f3d28743e307c7c4fed0bff6fead7ceca/src/main/java/org/apache/aurora/scheduler/SchedulerModule.java#L132-L147
> > 
> > https://github.com/apache/aurora/blob/89da936f3d28743e307c7c4fed0bff6fead7ceca/src/main/java/org/apache/aurora/scheduler/TierManager.java#L47-L55
> 
> Bill Farner wrote:
> Oh, the detail i neglected to mention - i suggest you then write code to 
> translate your config schema object into ExecutorSettings.  This can provides 
> compile-time guarantees that we don't have with the current code, making it 
> easier to maintain.
> 
> Renan DelValle wrote:
> Can you expend on this? I don't have any experience with Jackson. I'm 
> looking through what Maxim did and I'll definitely crib here and there but I 
> can't find anything related to getting this done at compile time.
> 
> Bill Farner wrote:
> Don't be thrown by the mention of compile time, i'm just referring to 
> refactor safety if/when the structures changed.  The concise way to put it is 
> - use a different class for your config domain object.  This way you don't 
> need to change ExecutorSettings to compensate for the fact that some code in 
> another part of the system is using a json mapper.
> 
> Start by defining a class that matches your config schema:
> ```
> class ExecutorConfig {
>   String command;
>   List resource;
>   ...
> }
> 
> class Resource {
>   String value;  // note - i would like to see this renamed to uri
>   boolean executable;
>   boolean extract;
>   boolean cache;
> }
> ...
> ```
> (that's just a quick example, feel free to adjust as you see fit)
> 
> Jackson is very similar to gson, there's just a behavior nuance that is 
> desirable.  You'll use an `ObjectMapper` to parse the json to an 
> `ExecutorConfig`, then write a function to transform your `ExecutorConfig` 
> into `ExecutorSettings`.
> 
> ```
> private static ExecutorSettings configToSettings(ExecutorConfig config) {
>   return new ExecutorSettings(...);
> }
> ```
> 
> Bill Farner wrote:
> I took a quick stab at illustrating this, take a look here: 
> https://github.com/wfarner/aurora/commit/a53c215432b34ba95238121581375bde24b88a1e
> 
> The remaining work is to complete filling out constructor fields:
> 
> https://github.com/wfarner/aurora/blob/a53c215432b34ba95238121581375bde24b88a1e/src/main/java/org/apache/aurora/scheduler/configuration/ExecutorSettingsLoader.java#L53-L61
> 
> This is really a matter of writing functions to translate from types in 
> `ExecutorConfig` to the corresponding types in `ExecutorSettings`.
> 
> Depending on your bandwidth over the next few days, i may take over this 
> patch if you don't mind.
> 
> Renan DelValle wrote:
> Thanks for the examples, in particular the entry transformer really 
> helped speed things along.
> 
> I've almost got this done, and I'm having an issue with using optionals, 
> which in my opinion are necessary if we want to leave the defaults set on the 
> URI resource configuration if nothing is specified. I.e: If executable is not 
> in the schema for a URI, then its better to not set it than to set a default 
> value, IMO.
> 
> I've looked around and Jackson has support for them if we load a new 
> module:
> https://github.com/FasterXML/jackson-datatype-jdk8
> 
> However, I can't find where the jackson dependecy is. And the fact the 
> jackson pacakges still point to codehaus lead me to believe were using an old 
> version.
> 
> Here are some relevant files:
> 
> https://github.com/rdelval/aurora/blob/AURORA-1376-1d/src/main/java/org/apache/aurora/scheduler/configuration/ExecutorConfiguration.java
> 
> https://github.com/rdelval/aurora/blob/AURORA-1376-1d/src/main/java/org/apache/aurora/scheduler/configuration/ExecutorSettingsLoader.java
> 
> We're almost there.
> 
> Bill Farner wrote:
> Perhaps we should go a step further and try to directly parse 
> `ExecutorInfo`?  That way we don't have to maintain parity between the 
> protobuf schema and our own.  Jackson can't directly dese

Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-09-21 Thread Aurora ReviewBot 

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


This patch does not apply cleanly on master (5627611), do you need to rebase?

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Sept. 21, 2015, 11:42 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37818/
> ---
> 
> (Updated Sept. 21, 2015, 11:42 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is the first stage in a series of patches to create support for custom 
> executors. In an effort to expedite the review process, I have decided to 
> break down my patch into multiple pieces that when/if commited won't break 
> the trunk.
> 
> This patch includes the ability to load configuration from a JSON file. A 
> JSON example file is included in examples/vagrant/executors-config.json
> 
> Command line arguments have been eliminated and moved over to the JSON file. 
> Jackson is used to deserialize a CommandInfo.Builder along with two other 
> fields. Volumes are now based on the Protobuf version and not the Aurora 
> Thrift version.
> 
> 
> Diffs
> -
> 
>   .gitignore 86840972c53ea52a793968d3d00df6763a7d6ffb 
>   3rdparty/python/requirements.txt 44217469a9583ec50233f34d54a32c105e6bab2c 
>   CHANGELOG 3b6990fb4dc3cfb8c730c81063413e505c803471 
>   NEWS a17f0e7c08fd30a0b2db6814a1c755111307228b 
>   README.md ef9c2f0ecf44b03e2025fe9cab62a3aa5ae30a77 
>   api/src/main/thrift/org/apache/aurora/gen/BUILD 
> fe3f83b6a7680985dce01efe2d54ccc4b0c2c482 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> d740a90e7732f42b43a79f8cf0afe705c061539c 
>   api/src/main/thrift/org/apache/aurora/gen/internal_rpc.thrift 
> a2c230fa9b5f648c4674042411cbe46fb8bb4faa 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> b7c4665d51f0efeebabd58c978707397b45d275d 
>   api/src/main/thrift/org/apache/thermos/BUILD 
> d0d789a6ee3971e3070f9397d53929563a77f7ea 
>   build-support/python/isort 44f9659948703c75372cd70643d5631acb116c2e 
>   build-support/python/isort-check 646cbf0bae4ccf8eac044817138ed1a7a59b261b 
>   build-support/python/make-mesos-native-egg 
> fce53e42dde72b893f291db9d78af1dc47fafe2f 
>   build-support/python/make-pycharm-virtualenv 
> 05a16d0d421982bcfb2e649be7b83a17d813efb1 
>   build-support/release/make-python-sdists 
> 9608f68e16243da01434ce2fc7d61bb7c7efd712 
>   build.gradle 700e1ade1470b99e9be390db150cf73aa06f17bc 
>   buildSrc/gradle.properties 3231a7add9e0503f0ee6779ae3c4000143b174ab 
>   buildSrc/src/main/groovy/org/apache/aurora/build/CoverageReportCheck.groovy 
> b996d4597492a76db0f2571db4e6c1ae9b4bcf33 
>   commons-args/src/main/java/org/apache/aurora/common/args/Arg.java 
> PRE-CREATION 
>   commons-args/src/main/java/org/apache/aurora/common/args/ArgParser.java 
> PRE-CREATION 
>   commons-args/src/main/java/org/apache/aurora/common/args/CmdLine.java 
> PRE-CREATION 
>   commons-args/src/main/java/org/apache/aurora/common/args/NoParser.java 
> PRE-CREATION 
>   commons-args/src/main/java/org/apache/aurora/common/args/Parser.java 
> PRE-CREATION 
>   commons-args/src/main/java/org/apache/aurora/common/args/ParserOracle.java 
> PRE-CREATION 
>   commons-args/src/main/java/org/apache/aurora/common/args/Positional.java 
> PRE-CREATION 
>   commons-args/src/main/java/org/apache/aurora/common/args/Verifier.java 
> PRE-CREATION 
>   commons-args/src/main/java/org/apache/aurora/common/args/VerifierFor.java 
> PRE-CREATION 
>   
> commons-args/src/main/java/org/apache/aurora/common/args/apt/CmdLineProcessor.java
>  PRE-CREATION 
>   
> commons-args/src/main/java/org/apache/aurora/common/args/apt/Configuration.java
>  PRE-CREATION 
>   
> commons-args/src/main/resources/META-INF/services/javax.annotation.processing.Processor
>  PRE-CREATION 
>   commons/src/main/java/org/apache/aurora/common/application/Lifecycle.java 
> PRE-CREATION 
>   
> commons/src/main/java/org/apache/aurora/common/application/ShutdownRegistry.java
>  PRE-CREATION 
>   
> commons/src/main/java/org/apache/aurora/common/application/ShutdownStage.java 
> PRE-CREATION 
>   commons/src/main/java/org/apache/aurora/common/args/ArgFilters.java 
> PRE-CREATION 
>   commons/src/main/java/org/apache/aurora/common/args/ArgScanner.java 
> PRE-CREATION 
>   commons/src/main/java/org/apache/aurora/common/args/Args.java PRE-CREATION 
>   commons/src/main/java/org/apache/aurora/common/args/ArgumentInfo.java 
> PRE-CREATION 
>   commons/src/main/java/org/apache/aurora/common/args/OptionInfo.java 
> PRE-CREATION 
>   commons/src/main/java/org/apache/aurora/common/args/Parsers.java 
> PRE-CREATI