Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread John Sirois 


> On Aug. 30, 2016, 11:56 a.m., Zameer Manji wrote:
> > LGTM modulo updating the docs about restarting all instances at the same 
> > time.
> > 
> > Could you also file a ticket to track the removal of the `zk_use_curator` 
> > flag in 0.17? I did not see one on JIRA.
> 
> John Sirois wrote:
> The language of https://issues.apache.org/jira/browse/AURORA-1669 tracks 
> the flag deprecatation and removal along with the removal of commons code.
> If that is too fat a ticket I can break a new one out.
> 
> Zameer Manji wrote:
> That ticket is fine, thanks for ensuring we don't drop this on the floor.

NP.  I bootstrapped the [0.17.0 release tracking 
ticket](https://issues.apache.org/jira/browse/AURORA-1760) and epics and linked 
this in.


- John


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


On Aug. 30, 2016, 2:25 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51506/
> ---
> 
> (Updated Aug. 30, 2016, 2:25 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1669
> https://issues.apache.org/jira/browse/AURORA-1669
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The flag is noted as deprecated for removal in 0.17.0.
> 
>  RELEASE-NOTES.md 
>|  2 ++
>  docs/reference/scheduler-configuration.md
>|  4 ++--
>  examples/vagrant/upstart/aurora-scheduler.conf   
>|  1 -
>  
> src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
>  | 11 +--
>  4 files changed, 13 insertions(+), 5 deletions(-)
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md f6b609f6f21d6dddfc45d79577b5070736e38ae2 
>   docs/reference/scheduler-configuration.md 
> c0c39442725c970e5f9811cd7b4ab3104364c671 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> dd60981564286e5b25546737fdd0ce08b0168ed4 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
>  36ad18c49c5693031136440ab163070f9ffa9405 
> 
> Diff: https://reviews.apache.org/r/51506/diff/
> 
> 
> Testing
> ---
> 
> Locally green:
> ```
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread Aurora ReviewBot 

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



Master (c99f2fb) is green with this patch.
  ./build-support/jenkins/build.sh

However, it appears that it might lack test coverage.

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Aug. 30, 2016, 8:25 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51506/
> ---
> 
> (Updated Aug. 30, 2016, 8:25 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1669
> https://issues.apache.org/jira/browse/AURORA-1669
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The flag is noted as deprecated for removal in 0.17.0.
> 
>  RELEASE-NOTES.md 
>|  2 ++
>  docs/reference/scheduler-configuration.md
>|  4 ++--
>  examples/vagrant/upstart/aurora-scheduler.conf   
>|  1 -
>  
> src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
>  | 11 +--
>  4 files changed, 13 insertions(+), 5 deletions(-)
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md f6b609f6f21d6dddfc45d79577b5070736e38ae2 
>   docs/reference/scheduler-configuration.md 
> c0c39442725c970e5f9811cd7b4ab3104364c671 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> dd60981564286e5b25546737fdd0ce08b0168ed4 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
>  36ad18c49c5693031136440ab163070f9ffa9405 
> 
> Diff: https://reviews.apache.org/r/51506/diff/
> 
> 
> Testing
> ---
> 
> Locally green:
> ```
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread Zameer Manji 


> On Aug. 30, 2016, 10:56 a.m., Zameer Manji wrote:
> > LGTM modulo updating the docs about restarting all instances at the same 
> > time.
> > 
> > Could you also file a ticket to track the removal of the `zk_use_curator` 
> > flag in 0.17? I did not see one on JIRA.
> 
> John Sirois wrote:
> The language of https://issues.apache.org/jira/browse/AURORA-1669 tracks 
> the flag deprecatation and removal along with the removal of commons code.
> If that is too fat a ticket I can break a new one out.

That ticket is fine, thanks for ensuring we don't drop this on the floor.


- Zameer


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


On Aug. 30, 2016, 1:25 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51506/
> ---
> 
> (Updated Aug. 30, 2016, 1:25 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1669
> https://issues.apache.org/jira/browse/AURORA-1669
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The flag is noted as deprecated for removal in 0.17.0.
> 
>  RELEASE-NOTES.md 
>|  2 ++
>  docs/reference/scheduler-configuration.md
>|  4 ++--
>  examples/vagrant/upstart/aurora-scheduler.conf   
>|  1 -
>  
> src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
>  | 11 +--
>  4 files changed, 13 insertions(+), 5 deletions(-)
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md f6b609f6f21d6dddfc45d79577b5070736e38ae2 
>   docs/reference/scheduler-configuration.md 
> c0c39442725c970e5f9811cd7b4ab3104364c671 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> dd60981564286e5b25546737fdd0ce08b0168ed4 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
>  36ad18c49c5693031136440ab163070f9ffa9405 
> 
> Diff: https://reviews.apache.org/r/51506/diff/
> 
> 
> Testing
> ---
> 
> Locally green:
> ```
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread John Sirois 

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

(Updated Aug. 30, 2016, 2:25 p.m.)


Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.


Changes
---

Finesse deprecation guidance.

 RELEASE-NOTES.md   
 | 9 +++--
 
src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java 
| 2 +-
 2 files changed, 8 insertions(+), 3 deletions(-)


Bugs: AURORA-1669
https://issues.apache.org/jira/browse/AURORA-1669


Repository: aurora


Description
---

The flag is noted as deprecated for removal in 0.17.0.

 RELEASE-NOTES.md   
 |  2 ++
 docs/reference/scheduler-configuration.md  
 |  4 ++--
 examples/vagrant/upstart/aurora-scheduler.conf 
 |  1 -
 
src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java 
| 11 +--
 4 files changed, 13 insertions(+), 5 deletions(-)


Diffs (updated)
-

  RELEASE-NOTES.md f6b609f6f21d6dddfc45d79577b5070736e38ae2 
  docs/reference/scheduler-configuration.md 
c0c39442725c970e5f9811cd7b4ab3104364c671 
  examples/vagrant/upstart/aurora-scheduler.conf 
dd60981564286e5b25546737fdd0ce08b0168ed4 
  
src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java 
36ad18c49c5693031136440ab163070f9ffa9405 

Diff: https://reviews.apache.org/r/51506/diff/


Testing
---

Locally green:
```
./gradlew -Pq build
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
```


Thanks,

John Sirois

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread Aurora ReviewBot 

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



This patch does not apply cleanly against master (c99f2fb), do you need to 
rebase?

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Aug. 30, 2016, 8 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51506/
> ---
> 
> (Updated Aug. 30, 2016, 8 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1669
> https://issues.apache.org/jira/browse/AURORA-1669
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The flag is noted as deprecated for removal in 0.17.0.
> 
>  RELEASE-NOTES.md 
>|  2 ++
>  docs/reference/scheduler-configuration.md
>|  4 ++--
>  examples/vagrant/upstart/aurora-scheduler.conf   
>|  1 -
>  
> src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
>  | 11 +--
>  4 files changed, 13 insertions(+), 5 deletions(-)
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 1819eaa20cf5014228643a1e120316d646cc2824 
>   docs/reference/scheduler-configuration.md 
> c0c39442725c970e5f9811cd7b4ab3104364c671 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> dd60981564286e5b25546737fdd0ce08b0168ed4 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
>  36ad18c49c5693031136440ab163070f9ffa9405 
> 
> Diff: https://reviews.apache.org/r/51506/diff/
> 
> 
> Testing
> ---
> 
> Locally green:
> ```
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread Joshua Cohen 

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


Ship it!




Ship It!

- Joshua Cohen


On Aug. 30, 2016, 8 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51506/
> ---
> 
> (Updated Aug. 30, 2016, 8 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1669
> https://issues.apache.org/jira/browse/AURORA-1669
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The flag is noted as deprecated for removal in 0.17.0.
> 
>  RELEASE-NOTES.md 
>|  2 ++
>  docs/reference/scheduler-configuration.md
>|  4 ++--
>  examples/vagrant/upstart/aurora-scheduler.conf   
>|  1 -
>  
> src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
>  | 11 +--
>  4 files changed, 13 insertions(+), 5 deletions(-)
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 1819eaa20cf5014228643a1e120316d646cc2824 
>   docs/reference/scheduler-configuration.md 
> c0c39442725c970e5f9811cd7b4ab3104364c671 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> dd60981564286e5b25546737fdd0ce08b0168ed4 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
>  36ad18c49c5693031136440ab163070f9ffa9405 
> 
> Diff: https://reviews.apache.org/r/51506/diff/
> 
> 
> Testing
> ---
> 
> Locally green:
> ```
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread John Sirois 

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

(Updated Aug. 30, 2016, 2 p.m.)


Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.


Changes
---

Finesse deprecation guidance.

 RELEASE-NOTES.md   
 | 9 +++--
 
src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java 
| 2 +-
 2 files changed, 8 insertions(+), 3 deletions(-)


Bugs: AURORA-1669
https://issues.apache.org/jira/browse/AURORA-1669


Repository: aurora


Description
---

The flag is noted as deprecated for removal in 0.17.0.

 RELEASE-NOTES.md   
 |  2 ++
 docs/reference/scheduler-configuration.md  
 |  4 ++--
 examples/vagrant/upstart/aurora-scheduler.conf 
 |  1 -
 
src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java 
| 11 +--
 4 files changed, 13 insertions(+), 5 deletions(-)


Diffs (updated)
-

  RELEASE-NOTES.md 1819eaa20cf5014228643a1e120316d646cc2824 
  docs/reference/scheduler-configuration.md 
c0c39442725c970e5f9811cd7b4ab3104364c671 
  examples/vagrant/upstart/aurora-scheduler.conf 
dd60981564286e5b25546737fdd0ce08b0168ed4 
  
src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java 
36ad18c49c5693031136440ab163070f9ffa9405 

Diff: https://reviews.apache.org/r/51506/diff/


Testing
---

Locally green:
```
./gradlew -Pq build
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
```


Thanks,

John Sirois

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread John Sirois 


> On Aug. 30, 2016, 12:23 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java,
> >  line 78
> > 
> >
> > I suggest against placing a version number into the warning message. 
> > We've had a few past examples when we did not follow up on deprecation 
> > warnings as promised.
> 
> Joshua Cohen wrote:
> +1 to this.

Removed.


- John


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


On Aug. 29, 2016, 5:37 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51506/
> ---
> 
> (Updated Aug. 29, 2016, 5:37 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1669
> https://issues.apache.org/jira/browse/AURORA-1669
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The flag is noted as deprecated for removal in 0.17.0.
> 
>  RELEASE-NOTES.md 
>|  2 ++
>  docs/reference/scheduler-configuration.md
>|  4 ++--
>  examples/vagrant/upstart/aurora-scheduler.conf   
>|  1 -
>  
> src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
>  | 11 +--
>  4 files changed, 13 insertions(+), 5 deletions(-)
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 1819eaa20cf5014228643a1e120316d646cc2824 
>   docs/reference/scheduler-configuration.md 
> c0c39442725c970e5f9811cd7b4ab3104364c671 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> dd60981564286e5b25546737fdd0ce08b0168ed4 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
>  36ad18c49c5693031136440ab163070f9ffa9405 
> 
> Diff: https://reviews.apache.org/r/51506/diff/
> 
> 
> Testing
> ---
> 
> Locally green:
> ```
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread John Sirois 


> On Aug. 30, 2016, 1:23 a.m., Stephan Erb wrote:
> > RELEASE-NOTES.md, lines 41-42
> > 
> >
> > You once recommended to stop all schedules at once when switching to 
> > Curator, rather than doing this in a rolling fashion. I suppose it is worth 
> > pointing out here as well.
> > 
> > I would also propose to move the part about the changed default value 
> > to the 'New/Updates' section.
> 
> Zameer Manji wrote:
> +1
> 
> I'm not sure what happens if you don't do that but it seems that a leader 
> won't be elected until all of them are on curator.

Done.


- John


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


On Aug. 29, 2016, 5:37 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51506/
> ---
> 
> (Updated Aug. 29, 2016, 5:37 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1669
> https://issues.apache.org/jira/browse/AURORA-1669
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The flag is noted as deprecated for removal in 0.17.0.
> 
>  RELEASE-NOTES.md 
>|  2 ++
>  docs/reference/scheduler-configuration.md
>|  4 ++--
>  examples/vagrant/upstart/aurora-scheduler.conf   
>|  1 -
>  
> src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
>  | 11 +--
>  4 files changed, 13 insertions(+), 5 deletions(-)
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 1819eaa20cf5014228643a1e120316d646cc2824 
>   docs/reference/scheduler-configuration.md 
> c0c39442725c970e5f9811cd7b4ab3104364c671 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> dd60981564286e5b25546737fdd0ce08b0168ed4 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
>  36ad18c49c5693031136440ab163070f9ffa9405 
> 
> Diff: https://reviews.apache.org/r/51506/diff/
> 
> 
> Testing
> ---
> 
> Locally green:
> ```
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread John Sirois 


> On Aug. 30, 2016, 11:56 a.m., Zameer Manji wrote:
> > LGTM modulo updating the docs about restarting all instances at the same 
> > time.
> > 
> > Could you also file a ticket to track the removal of the `zk_use_curator` 
> > flag in 0.17? I did not see one on JIRA.

The language of https://issues.apache.org/jira/browse/AURORA-1669 tracks the 
flag deprecatation and removal along with the removal of commons code.
If that is too fat a ticket I can break a new one out.


- John


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


On Aug. 29, 2016, 5:37 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51506/
> ---
> 
> (Updated Aug. 29, 2016, 5:37 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1669
> https://issues.apache.org/jira/browse/AURORA-1669
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The flag is noted as deprecated for removal in 0.17.0.
> 
>  RELEASE-NOTES.md 
>|  2 ++
>  docs/reference/scheduler-configuration.md
>|  4 ++--
>  examples/vagrant/upstart/aurora-scheduler.conf   
>|  1 -
>  
> src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
>  | 11 +--
>  4 files changed, 13 insertions(+), 5 deletions(-)
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 1819eaa20cf5014228643a1e120316d646cc2824 
>   docs/reference/scheduler-configuration.md 
> c0c39442725c970e5f9811cd7b4ab3104364c671 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> dd60981564286e5b25546737fdd0ce08b0168ed4 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
>  36ad18c49c5693031136440ab163070f9ffa9405 
> 
> Diff: https://reviews.apache.org/r/51506/diff/
> 
> 
> Testing
> ---
> 
> Locally green:
> ```
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread Joshua Cohen 


> On Aug. 30, 2016, 6:23 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java,
> >  line 78
> > 
> >
> > I suggest against placing a version number into the warning message. 
> > We've had a few past examples when we did not follow up on deprecation 
> > warnings as promised.

+1 to this.


- Joshua


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


On Aug. 29, 2016, 11:37 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51506/
> ---
> 
> (Updated Aug. 29, 2016, 11:37 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1669
> https://issues.apache.org/jira/browse/AURORA-1669
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The flag is noted as deprecated for removal in 0.17.0.
> 
>  RELEASE-NOTES.md 
>|  2 ++
>  docs/reference/scheduler-configuration.md
>|  4 ++--
>  examples/vagrant/upstart/aurora-scheduler.conf   
>|  1 -
>  
> src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
>  | 11 +--
>  4 files changed, 13 insertions(+), 5 deletions(-)
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 1819eaa20cf5014228643a1e120316d646cc2824 
>   docs/reference/scheduler-configuration.md 
> c0c39442725c970e5f9811cd7b4ab3104364c671 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> dd60981564286e5b25546737fdd0ce08b0168ed4 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
>  36ad18c49c5693031136440ab163070f9ffa9405 
> 
> Diff: https://reviews.apache.org/r/51506/diff/
> 
> 
> Testing
> ---
> 
> Locally green:
> ```
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread Maxim Khutornenko 

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




src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java 
(line 78)


I suggest against placing a version number into the warning message. We've 
had a few past examples when we did not follow up on deprecation warnings as 
promised.


- Maxim Khutornenko


On Aug. 29, 2016, 11:37 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51506/
> ---
> 
> (Updated Aug. 29, 2016, 11:37 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1669
> https://issues.apache.org/jira/browse/AURORA-1669
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The flag is noted as deprecated for removal in 0.17.0.
> 
>  RELEASE-NOTES.md 
>|  2 ++
>  docs/reference/scheduler-configuration.md
>|  4 ++--
>  examples/vagrant/upstart/aurora-scheduler.conf   
>|  1 -
>  
> src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
>  | 11 +--
>  4 files changed, 13 insertions(+), 5 deletions(-)
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 1819eaa20cf5014228643a1e120316d646cc2824 
>   docs/reference/scheduler-configuration.md 
> c0c39442725c970e5f9811cd7b4ab3104364c671 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> dd60981564286e5b25546737fdd0ce08b0168ed4 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
>  36ad18c49c5693031136440ab163070f9ffa9405 
> 
> Diff: https://reviews.apache.org/r/51506/diff/
> 
> 
> Testing
> ---
> 
> Locally green:
> ```
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread Zameer Manji 

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


Ship it!




LGTM modulo updating the docs about restarting all instances at the same time.

Could you also file a ticket to track the removal of the `zk_use_curator` flag 
in 0.17? I did not see one on JIRA.

- Zameer Manji


On Aug. 29, 2016, 4:37 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51506/
> ---
> 
> (Updated Aug. 29, 2016, 4:37 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1669
> https://issues.apache.org/jira/browse/AURORA-1669
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The flag is noted as deprecated for removal in 0.17.0.
> 
>  RELEASE-NOTES.md 
>|  2 ++
>  docs/reference/scheduler-configuration.md
>|  4 ++--
>  examples/vagrant/upstart/aurora-scheduler.conf   
>|  1 -
>  
> src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
>  | 11 +--
>  4 files changed, 13 insertions(+), 5 deletions(-)
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 1819eaa20cf5014228643a1e120316d646cc2824 
>   docs/reference/scheduler-configuration.md 
> c0c39442725c970e5f9811cd7b4ab3104364c671 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> dd60981564286e5b25546737fdd0ce08b0168ed4 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
>  36ad18c49c5693031136440ab163070f9ffa9405 
> 
> Diff: https://reviews.apache.org/r/51506/diff/
> 
> 
> Testing
> ---
> 
> Locally green:
> ```
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread Zameer Manji 


> On Aug. 30, 2016, 12:23 a.m., Stephan Erb wrote:
> > RELEASE-NOTES.md, lines 41-42
> > 
> >
> > You once recommended to stop all schedules at once when switching to 
> > Curator, rather than doing this in a rolling fashion. I suppose it is worth 
> > pointing out here as well.
> > 
> > I would also propose to move the part about the changed default value 
> > to the 'New/Updates' section.

+1

I'm not sure what happens if you don't do that but it seems that a leader won't 
be elected until all of them are on curator.


- Zameer


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


On Aug. 29, 2016, 4:37 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51506/
> ---
> 
> (Updated Aug. 29, 2016, 4:37 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1669
> https://issues.apache.org/jira/browse/AURORA-1669
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The flag is noted as deprecated for removal in 0.17.0.
> 
>  RELEASE-NOTES.md 
>|  2 ++
>  docs/reference/scheduler-configuration.md
>|  4 ++--
>  examples/vagrant/upstart/aurora-scheduler.conf   
>|  1 -
>  
> src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
>  | 11 +--
>  4 files changed, 13 insertions(+), 5 deletions(-)
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 1819eaa20cf5014228643a1e120316d646cc2824 
>   docs/reference/scheduler-configuration.md 
> c0c39442725c970e5f9811cd7b4ab3104364c671 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> dd60981564286e5b25546737fdd0ce08b0168ed4 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
>  36ad18c49c5693031136440ab163070f9ffa9405 
> 
> Diff: https://reviews.apache.org/r/51506/diff/
> 
> 
> Testing
> ---
> 
> Locally green:
> ```
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread Stephan Erb 

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


Fix it, then Ship it!





RELEASE-NOTES.md (lines 41 - 42)


You once recommended to stop all schedules at once when switching to 
Curator, rather than doing this in a rolling fashion. I suppose it is worth 
pointing out here as well.

I would also propose to move the part about the changed default value to 
the 'New/Updates' section.


- Stephan Erb


On Aug. 30, 2016, 1:37 a.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51506/
> ---
> 
> (Updated Aug. 30, 2016, 1:37 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1669
> https://issues.apache.org/jira/browse/AURORA-1669
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The flag is noted as deprecated for removal in 0.17.0.
> 
>  RELEASE-NOTES.md 
>|  2 ++
>  docs/reference/scheduler-configuration.md
>|  4 ++--
>  examples/vagrant/upstart/aurora-scheduler.conf   
>|  1 -
>  
> src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
>  | 11 +--
>  4 files changed, 13 insertions(+), 5 deletions(-)
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 1819eaa20cf5014228643a1e120316d646cc2824 
>   docs/reference/scheduler-configuration.md 
> c0c39442725c970e5f9811cd7b4ab3104364c671 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> dd60981564286e5b25546737fdd0ce08b0168ed4 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
>  36ad18c49c5693031136440ab163070f9ffa9405 
> 
> Diff: https://reviews.apache.org/r/51506/diff/
> 
> 
> Testing
> ---
> 
> Locally green:
> ```
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-29 Thread Aurora ReviewBot 

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



Master (c34f78a) is green with this patch.
  ./build-support/jenkins/build.sh

However, it appears that it might lack test coverage.

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Aug. 29, 2016, 11:37 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51506/
> ---
> 
> (Updated Aug. 29, 2016, 11:37 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1669
> https://issues.apache.org/jira/browse/AURORA-1669
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The flag is noted as deprecated for removal in 0.17.0.
> 
>  RELEASE-NOTES.md 
>|  2 ++
>  docs/reference/scheduler-configuration.md
>|  4 ++--
>  examples/vagrant/upstart/aurora-scheduler.conf   
>|  1 -
>  
> src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
>  | 11 +--
>  4 files changed, 13 insertions(+), 5 deletions(-)
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 1819eaa20cf5014228643a1e120316d646cc2824 
>   docs/reference/scheduler-configuration.md 
> c0c39442725c970e5f9811cd7b4ab3104364c671 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> dd60981564286e5b25546737fdd0ce08b0168ed4 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
>  36ad18c49c5693031136440ab163070f9ffa9405 
> 
> Diff: https://reviews.apache.org/r/51506/diff/
> 
> 
> Testing
> ---
> 
> Locally green:
> ```
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>