You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Joshua Cohen <jc...@apache.org> on 2016/03/22 19:53:45 UTC
Review Request 45172: Tweak update-sources script to also update mesos
config.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45172/
-----------------------------------------------------------
Review request for Aurora and Bill Farner.
Repository: aurora
Description
-------
Tweak update-sources script to also update mesos config.
Diffs
-----
examples/vagrant/provision-dev-cluster.sh b1f661ea90ac427621518f653cce1b7630d2a6ed
Diff: https://reviews.apache.org/r/45172/diff/
Testing
-------
Thanks,
Joshua Cohen
Re: Review Request 45172: Tweak update-sources script to also update
mesos config.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45172/#review124868
-----------------------------------------------------------
Ship it!
Master (d5d7ec0) 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 March 22, 2016, 6:53 p.m., Joshua Cohen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45172/
> -----------------------------------------------------------
>
> (Updated March 22, 2016, 6:53 p.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Tweak update-sources script to also update mesos config.
>
>
> Diffs
> -----
>
> examples/vagrant/provision-dev-cluster.sh b1f661ea90ac427621518f653cce1b7630d2a6ed
>
> Diff: https://reviews.apache.org/r/45172/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joshua Cohen
>
>
Re: Review Request 45172: Tweak update-sources script to also update
mesos config.
Posted by Joshua Cohen <jc...@apache.org>.
> On March 23, 2016, 3:26 a.m., Bill Farner wrote:
> > examples/vagrant/provision-dev-cluster.sh, lines 89-92
> > <https://reviews.apache.org/r/45172/diff/1/?file=1311002#file1311002line89>
> >
> > Echoing IRC discussion: i'm -1 to this change because it copies sources but does not effect the change by restarting components. You rightly point out that it's no different from the copy we do for the upstart configs.
> >
> > My vote is we do one of the following:
> > 1. move the upstart config copy out of `update-sources` and do it only at provision time
> > 2. create a helper function that accepts 3 args: `src`, `dest`, `on_change_cmd`, which will do an `rsync` and invoke `on_change_cmd` if any files were changed. use this to restart components that were reconfigured
> >
> > I think there's not much effort in (2), but it's objectively less work than (1). What do you think?
Thanks for commenting here, I came to capture the IRC discussion and found you had already done it.
Can I propose a third option: what if we did something like:
diff /vagrant/examples/vagrant/mesos_config/etc_mesos-slave /etc/mesos-slave && \
cp /vagrant/examples/vagrant/mesos_config/etc_mesos-slave/* /etc/mesos-slave && \
sudo restart mesos-slave
I guess, now that I think about it, this is essentially what you're suggesting as option 2 above... so, I guess I'm ok with option 2 ;).
- Joshua
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45172/#review124956
-----------------------------------------------------------
On March 22, 2016, 6:53 p.m., Joshua Cohen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45172/
> -----------------------------------------------------------
>
> (Updated March 22, 2016, 6:53 p.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Tweak update-sources script to also update mesos config.
>
>
> Diffs
> -----
>
> examples/vagrant/provision-dev-cluster.sh b1f661ea90ac427621518f653cce1b7630d2a6ed
>
> Diff: https://reviews.apache.org/r/45172/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joshua Cohen
>
>
Re: Review Request 45172: Tweak update-sources script to also update
mesos config.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45172/#review124956
-----------------------------------------------------------
examples/vagrant/provision-dev-cluster.sh (lines 89 - 92)
<https://reviews.apache.org/r/45172/#comment187663>
Echoing IRC discussion: i'm -1 to this change because it copies sources but does not effect the change by restarting components. You rightly point out that it's no different from the copy we do for the upstart configs.
My vote is we do one of the following:
1. move the upstart config copy out of `update-sources` and do it only at provision time
2. create a helper function that accepts 3 args: `src`, `dest`, `on_change_cmd`, which will do an `rsync` and invoke `on_change_cmd` if any files were changed. use this to restart components that were reconfigured
I think there's not much effort in (2), but it's objectively less work than (1). What do you think?
- Bill Farner
On March 22, 2016, 11:53 a.m., Joshua Cohen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45172/
> -----------------------------------------------------------
>
> (Updated March 22, 2016, 11:53 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Tweak update-sources script to also update mesos config.
>
>
> Diffs
> -----
>
> examples/vagrant/provision-dev-cluster.sh b1f661ea90ac427621518f653cce1b7630d2a6ed
>
> Diff: https://reviews.apache.org/r/45172/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joshua Cohen
>
>