mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Till Toenshoff <toensh...@me.com>
Subject Re: Review Request 47821: Remove SASL dependency for Windows builds.
Date Wed, 25 May 2016 21:31:40 GMT

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



Thanks for proposing this workaround for the SASL on Windows challenge. I must confess that
I am not really *happy* with this but given our constraints, I believe the way you reduced
my original patch is indeed least painful ;). So it looks good for me, barring the needed
comment rewording.


cmake/CompilationConfigure.cmake (lines 132 - 139)
<https://reviews.apache.org/r/47821/#comment199772>

    Could you please reword this (and the other) comment to reflect the fact that we do have
a pluggable authentication layer?
    
    I did add some clues for such reword into the linked JIRA issue. Given that I am not a
native speaker, I guess you can do a much better job than I ever could for such updated comment
:)
    Please note that we commonly have the "link" to the JIRA issue as trailing sentence; "...
See MESOS-XXXX."


- Till Toenshoff


On May 25, 2016, 6:45 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47821/
> -----------------------------------------------------------
> 
> (Updated May 25, 2016, 6:45 p.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van Remoortere, and
Michael Park.
> 
> 
> Bugs: MESOS-5450
>     https://issues.apache.org/jira/browse/MESOS-5450
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> SASL is currently a hard dependency for Mesos. Per MESOS-5450, we expect
> that some platforms will not support SASL (namely Windows), so this
> commit will begin the first step in a several-step process of removing
> it as a hard dependency.
> 
> For this first step, this commit will shed SASL dependency in libmesos
> only for Windows builds. We do this by:
> 
>   (1) Adding a preprocessor symbol, `HAS_AUTHENTICATION` that wraps the
>       SASL-dependent code, so that we can conditionally choose not to
>       compile it.
>   (2) Defining `HAS_AUTHENTICATION` on all Unix builds, and leaving it
>       undefined on all Windows builds.
>   (3) Logging an error and exiting the master if the user passes in
>       flags that depend on SASL, such as `--authenticate`.
> 
> Notably, what we do *not* do is:
> 
>   (1) Shed SASL dependency in the tests.
> 
> The impact of this is that, on Windows, relevant libmesos tests will
> either not compile, or not pass. Naturally, as tracked by MESOS-5450,
> the second phase of this series will be to shed the test dependency as
> well.
> 
> 
> Diffs
> -----
> 
>   cmake/CompilationConfigure.cmake 5c7833ceaed556cc4ffb650996e918c1a542c5f0 
>   src/Makefile.am 447bc2ab511ad173d3d911be10992be0974f4584 
>   src/master/master.cpp 35b428b0f7dee5954514d8860cfc498271ccf267 
>   src/sched/sched.cpp 4693d0dc09afc3ddbbf34e166579b6a6d71c3e38 
> 
> Diff: https://reviews.apache.org/r/47821/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>


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