aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mark Chu-Carroll" <mchucarr...@twopensource.com>
Subject Re: Review Request 23199: Log loaded config file at level TRANSCRIPT (aka INFO+1)
Date Tue, 01 Jul 2014 18:14:52 GMT


> On July 1, 2014, 1:55 p.m., Kevin Sweeney wrote:
> > src/main/python/apache/aurora/client/cli/context.py, lines 63-64
> > <https://reviews.apache.org/r/23199/diff/1/?file=621349#file621349line63>
> >
> >     This doesn't play nicely with the include() directive, which may load other
config files from elsewhere. It also introduces a race (you open and close the config file
here, then the loader presumably does as well meaning that the file contents could change
after you've logged them). I wonder if this logic would be better pushed down to the loader.

(1) Race condition really isn't an issue here. If a user is modifying a config file while
aurora is running, the results will (at best) be flaky. This doesn't make that any worse.

(2) I'd like to put it in the loader, but it's not feasible. The loader is, ultimately, inside
of pystachio (via a complex route: get_config -> AnnotatedAuroraConfig.load -> AuroraConfig.load
-> AuroraConfigLoader.load -> AuroraConfigLoader.load_raw -> PystachioConfig.__apply_)

(3) This doesn't follow include semantics - but it's not trying to solve that problem. The
way that the aurora docs read, the config file should be at the path specified on the command-line.
If it's not, it should be an error. For this level of logging, I'm not even trying to resolve
includes - just simply dumping file contents.


> On July 1, 2014, 1:55 p.m., Kevin Sweeney wrote:
> > src/main/python/apache/aurora/client/cli/context.py, line 64
> > <https://reviews.apache.org/r/23199/diff/1/?file=621349#file621349line64>
> >
> >     Add a constant for the TRANSCRIPT loglevel somewhere?

There is, but using it here would create a circular dependency.


- Mark


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


On July 1, 2014, 11:03 a.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23199/
> -----------------------------------------------------------
> 
> (Updated July 1, 2014, 11:03 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: aurora-567
>     https://issues.apache.org/jira/browse/aurora-567
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Have client commands that load config files print the contents of the loaded config file
to 
> log at level TRANSCRIPT (aka INFO+1).
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/client/cli/context.py facd52c14e330c35889cec0292bb380cecff9641

>   src/test/python/apache/aurora/client/cli/test_logging.py b3c3b8deaa8961251a1be8121659e742b00f6df2

> 
> Diff: https://reviews.apache.org/r/23199/diff/
> 
> 
> Testing
> -------
> 
> Added unit test of new functionality, ran all tests.
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message