mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vinod Kone <vinodk...@gmail.com>
Subject Re: Review Request 60439: Added scripts to automate website publishing.
Date Thu, 27 Jul 2017 00:28:38 GMT


> On July 4, 2017, 12:02 p.m., Benjamin Bannier wrote:
> > This is a great start, but I would prefer if we wouldn't directly mount an existing
source directory into the container and bootstrap, configure and build in it (this is also
broken for e.g., in-source builds).
> > 
> > What about instead modify mounting the Mesos source tree into the container, then
inside the container cloning from it, and building in the clean checkout? This woudl not only
minimize the risk of destructive side-effects outside the container, but also e.g., make sure
that we do not by accident pick up uncommitted changes. This is the approach we also followed
in `support/mesos-tidy`.
> > 
> > I left some first comments, but will look again once the handling of the source
tree is under control.

Good point.

This script was originally intended to be used by CI and not by human users. The CI job does
a clean checkout of mesos source and site repos, so there shouldn't be any problem with picking
up unexpected uncommitted changes from mesos source repo. If this script will be eventually
used by humans as well, as suggested in the next review, then yes I agree we need to be more
careful. Although for dev workflow case, it might actually be preferrable that the website
dev build picks up uncommitted changes while testing; otherwise users need to commit their
code to see how it looks on website while dev/testing, which is not a great experience IMO.


So I would like to punt on this for now until we figure out merging the dev and publish workflows.


> On July 4, 2017, 12:02 p.m., Benjamin Bannier wrote:
> > support/jenkins/websitebot.sh
> > Lines 31 (patched)
> > <https://reviews.apache.org/r/60439/diff/1/?file=1763077#file1763077line31>
> >
> >     If the source tree contains uncommitted changes, the output will not reproducibly
correspond to this SHA1.
> >     
> >     I'd suggest to instead clone the source tree instead.

see my comment above.


> On July 4, 2017, 12:02 p.m., Benjamin Bannier wrote:
> > support/mesos-website.sh
> > Lines 33 (patched)
> > <https://reviews.apache.org/r/60439/diff/1/?file=1763078#file1763078line33>
> >
> >     Not crucial here right now, but let's put this string into single quotes, see
https://github.com/koalaman/shellcheck/wiki/SC2064.
> >     
> >     Also please consider working on getting this image published in the Mesos dockerhub
org so we can pull from there instead of having the rebuild the same image over and over again.
See e.g., `support/mesos-tidy.sh` how we already do this for other images.

Yea, I very much liked to use a image hosted on dockerhub; but one thing at a time :)


> On July 4, 2017, 12:02 p.m., Benjamin Bannier wrote:
> > support/mesos-website/Dockerfile
> > Lines 1-11 (patched)
> > <https://reviews.apache.org/r/60439/diff/1/?file=1763079#file1763079line1>
> >
> >     We now have one Docker image to run tests in CI (via `docker_build.sh`), one
to run clang-tidy checks (`mesos-tidy/Dockerfile`), and are adding another one  here to build
the website.
> >     
> >     We should try to reduce the number of these images or remove duplication, e.g.,
this image could be in principle implemented as a small addon on top of `mesos/mesos-tidy`
> >     
> >         FROM mesos/mesos-tidy:latest
> >         LABEL Description="This image is used for generating Mesos web site."
> >         
> >         # Install ruby and doxygen tools ...

I agree. We'll fix it as part of https://issues.apache.org/jira/browse/MESOS-6984


- Vinod


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


On June 29, 2017, 9:59 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60439/
> -----------------------------------------------------------
> 
> (Updated June 29, 2017, 9:59 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and haosdent huang.
> 
> 
> Bugs: MESOS-7625
>     https://issues.apache.org/jira/browse/MESOS-7625
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> These scripts are expected to be run by ASF CI on any commit to
> the mesos repo. The directory layout is similar to tidybot CI job.
> 
> 
> Diffs
> -----
> 
>   support/jenkins/websitebot.sh PRE-CREATION 
>   support/mesos-website.sh PRE-CREATION 
>   support/mesos-website/Dockerfile PRE-CREATION 
>   support/mesos-website/build.sh PRE-CREATION 
>   support/mesos-website/entrypoint.sh PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60439/diff/1/
> 
> 
> Testing
> -------
> 
> Tested by running a CI job pointing to a branch containing this patch.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


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