> On March 8, 2016, 7:25 p.m., Zameer Manji wrote:
> > The code for this approach looks fine to me, but I'm not sure if this 
> > approach is the way to go.
> > 
> > Why can't the command for the health checker include 
> > '{{thermos.ports[http]}}' and we can resolve that value before launching 
> > the subprocess? Thats more consistent with the rest of the DSL. Further, 
> > using the mustache variables in the command variable would allow the health 
> > checker process to have access to all of the same information that task 
> > processes have like hostname.
> > 
> > For example the command could be '/usr/bin/health_checker 
> > --port-to-check={{thermos.ports[http]}}'.
> 
> Joshua Cohen wrote:
>     I think this is an excellent point, good catch Zameer!
>     
>     Dmitriy, is there any reason why this approach won't work for you guys?

Yea, I tried using that approach at first. But we need to allocate 10 ports 
(mix of HTTP and another RPC protocol(s)) for some services, and our existing 
health check scripts at the moment are just simple bash scripts. Dealing with 
10 arguments will become difficult (imagine writing one for that case), 
especially since order will matter and the owner will need to keep track of 
order in which they are passed in (eg is it an HTTP one, or some RPC protocol.. 
oh wait, I thought that was passed in first... dammit I have to read code in 
how our internal system massages them since we bypass aurora client completely 
to see how it actually works). Environment variables are just easier to deal 
with. 

Does this make sense? Perhaps I could have explained why this approach in the 
Summary/Description.


- Dmitriy


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


On March 8, 2016, 6:32 p.m., Dmitriy Shirchenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44486/
> -----------------------------------------------------------
> 
> (Updated March 8, 2016, 6:32 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1622
>     https://issues.apache.org/jira/browse/AURORA-1622
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Exposing ports to shell health checkers
> 
> 
> Diffs
> -----
> 
>   NEWS b84a94550f93691eba0220afedb2bb4d5e00e6bd 
>   docs/configuration-reference.md 10702ff4e700b6da7bdd7fd036de442be1eba45c 
>   src/main/python/apache/aurora/common/health_check/shell.py 
> 890bf0c5d50d0022c044a37191a2e3145cc6340f 
>   src/main/python/apache/aurora/executor/common/health_checker.py 
> 303972778baa04e9d7dd47fb208fe1427e779976 
>   src/test/python/apache/aurora/common/health_check/test_shell.py 
> 84f717fbf724c11863b4980fd2740dc23fe1404e 
>   src/test/python/apache/aurora/executor/common/test_health_checker.py 
> 9bebce8f5a26662f58075d7ce881a8bdacb2fe46 
> 
> Diff: https://reviews.apache.org/r/44486/diff/
> 
> 
> Testing
> -------
> 
> Unit and end to end test.
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>

Reply via email to