Re: Review Request 62590: WIP: Update to Thrift 0.10.0

2017-12-03 Thread John Sirois


> On Sept. 26, 2017, 5:53 p.m., Bill Farner wrote:
> > ```
> > /bin/sh: cmake: command not found
> > ```
> > 
> > But now i need to install cmake, so i'm not sure this pays off.
> 
> Bill Farner wrote:
> (this = the switch to cmake)
> 
> Stephan Erb wrote:
> Bison on MacOs is 10 years old. I assumed they have a good reason for it 
> and considered an added dependency a less risky endeavor. (Or is it just 
> negligence?)
> 
> Bill Farner wrote:
> What i meant to say - currently, i can build aurora on a stock macOS 
> machine with only a modern JDK and xcode command line utilities.  Thrift 
> 0.10.0 seems to put us in the position of choosing between:
> 
> a.) adding another step to our bootstrap routine to pre-build bison
> b.) adding bison as a build-time dependency
> c.) adding cmake as a build-time dependency
> d.) other options? (i'd like to float the idea of hosting thrift binaries 
> like how pants does)
> 
> Stephan Erb wrote:
> Giving it some thought, you are right that just requring cmake does not 
> improve the situation in any way.
> 
> I also like that we can build Aurora on a stock MacOS without much hassle 
> (or administrator rights). I think we should retain this property. This would 
> restrict us to options a) and d).
> 
> Bill Farner wrote:
> I would support (d).  We can place unofficial binaries in svn for dev 
> platforms as needed, and could support using thrift from the `PATH` as a 
> fallback.  This has the bonus of making from-scratch builds much faster.
> 
> Stephan Erb wrote:
> I saw your patch to https://github.com/morimekta/providence. Did you 
> manage to get something working with it?
> 
> Bill Farner wrote:
> I'm about 20% along.  A very large mechanical patch is needed for Aurora. 
>  The only untested piece is binary format compatibility (which providence 
> aims to achieve).  There is some incompatibility i need to investigate 
> further (trivial details like a round-tripped `null` collection turning into 
> an empty collection), but so far it looks good!  I'm proceeding with high 
> confidence, as there is also the future promise of a more approachable 
> HTTP/JSON interface via thrift IDL when using providence.
> 
> Bill Farner wrote:
> I should have clarified - i don't think we should hold back on upgrading 
> thrift.  It's not yet guaranteed that the migration to providence will be 
> successful or timely.
> 
> As for my proposed (d) above, we could store prebuilt binaries similarly 
> to how we store mesos eggs: https://svn.apache.org/repos/asf/aurora/3rdparty/
> 
> Stephan Erb wrote:
> I have just pushed another patch that I have been sitting one for a few 
> days. It suffers from the same pants issues as 
> https://reviews.apache.org/r/63750/

The still incomplete solution: https://reviews.apache.org/r/64290/


- John


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


On Dec. 3, 2017, 1:09 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62590/
> ---
> 
> (Updated Dec. 3, 2017, 1:09 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Update to Thrift 0.10.0 (https://github.com/apache/thrift/blob/master/CHANGES)
> 
> Included changes:
> 
> * Update to the latest pants version. This was necesary to make `./pants gen` 
> working. Unfortunately this breaks a few things. For details see 
> https://reviews.apache.org/r/63750/ 
> * The Java `hashcode` option has been removed as it is now the default.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 155930dc6b808fb2f573f427a4508e388ee04b5a 
>   build-support/packer/build.sh 85444125abc0c7e600a09933411e57c0d74051ac 
>   build-support/thrift/AURORA-1727.compiler.cpp.Makefile.in.patch 
> 77c966caa3d1f644241bcc2b1968bc9306c56689 
>   
> build-support/thrift/AURORA-1727.compiler.cpp.src.generate.t_java_generator.cc.patch
>  42300b43a8f72e45c96b975e5d3a6a7bd0283529 
>   build-support/thrift/AURORA-1727.lib.py.setup.py.patch 
> 11c7b13341e156f3686511cb40ab13c1256203a6 
>   build-support/thrift/BUILD ab19f1f68682d88f731a463c15591e45a317e760 
>   build-support/thrift/Makefile f440b610afe321af663e393a29eebda7af7bd7a8 
>   build-support/thrift/prepare_binary.sh 
> 4ad997bf039294f7940b93a76ebf014689f8f618 
>   build-support/thrift/thriftw c8debd07bc9da97fb58db795e67c9ac82cc30bc1 
>   build.gradle af119910e84c48f75f2573ababcfa287c3b986fc 
>   buildSrc/src/main/groovy/org/apache/aurora/build/ThriftPlugin.groovy 
> fc2bc9082dae2c63aa578c05dc89feb346260a67 
>   pants.ini 0671d9ab6381e5b9c324dc09a891a639cbfb2ccc 
>   

Re: Review Request 62590: WIP: Update to Thrift 0.10.0

2017-12-03 Thread Aurora ReviewBot

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



Master (89338dd) is red with this patch.
  ./build-support/jenkins/build.sh

20:23:58 00:00   [setup]
20:23:58 00:00 [parse]
   Executing tasks in goals: gen -> pyprep -> test
20:23:58 00:00   [gen]
20:23:58 00:00 [thrift-py]
20:23:58 00:00   [cache] 
   No cached artifacts for 1 target.
   Invalidated 1 target..
20:23:58 00:00   [pyprep]
20:23:58 00:00 [interpreter]
20:24:02 00:04 [requirements]
20:24:02 00:04   [cache]  
   No cached artifacts for 34 targets.
   Invalidated 34 targets. Failed to install 
pykerberos-1.1.14 (caused by: NonZeroExit("received exit code 1 during 
execution of `[u'/usr/bin/python2.7', '-', 'bdist_wheel', 
'--dist-dir=/tmp/tmpVbU5Nf']` while trying to execute `[u'/usr/bin/python2.7', 
'-', 'bdist_wheel', '--dist-dir=/tmp/tmpVbU5Nf']`",)
):
stdout:
running bdist_wheel
running build
running build_ext
building 'kerberos' extension
creating build
creating build/temp.linux-x86_64-2.7
creating build/temp.linux-x86_64-2.7/src
x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes 
-fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g 
-fstack-protector-strong -Wformat -Werror=format-security -fPIC 
-I/usr/include/python2.7 -c src/kerberos.c -o 
build/temp.linux-x86_64-2.7/src/kerberos.o

stderr:
In file included from src/kerberos.c:19:0:
src/kerberosbasic.h:17:27: fatal error: gssapi/gssapi.h: No such file or 
directory
compilation terminated.
error: command 'x86_64-linux-gnu-gcc' failed with exit status 1



   Waiting for background workers to finish.
20:24:21 00:23   [complete]
   FAILURE
Exception caught: ()

Exception message: Package 
SourcePackage(u'file:///home/jenkins/jenkins-slave/workspace/AuroraBot/.pants.d/python-setup/resolved_requirements/CPython-2.7.12/pykerberos-1.1.14.tar.gz')
 is not translateable by ChainedTranslator(WheelTranslator, EggTranslator, 
SourceTranslator)



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

- Aurora ReviewBot


On Dec. 3, 2017, 12:09 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62590/
> ---
> 
> (Updated Dec. 3, 2017, 12:09 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Update to Thrift 0.10.0 (https://github.com/apache/thrift/blob/master/CHANGES)
> 
> Included changes:
> 
> * Update to the latest pants version. This was necesary to make `./pants gen` 
> working. Unfortunately this breaks a few things. For details see 
> https://reviews.apache.org/r/63750/ 
> * The Java `hashcode` option has been removed as it is now the default.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 155930dc6b808fb2f573f427a4508e388ee04b5a 
>   build-support/packer/build.sh 85444125abc0c7e600a09933411e57c0d74051ac 
>   build-support/thrift/AURORA-1727.compiler.cpp.Makefile.in.patch 
> 77c966caa3d1f644241bcc2b1968bc9306c56689 
>   
> build-support/thrift/AURORA-1727.compiler.cpp.src.generate.t_java_generator.cc.patch
>  42300b43a8f72e45c96b975e5d3a6a7bd0283529 
>   build-support/thrift/AURORA-1727.lib.py.setup.py.patch 
> 11c7b13341e156f3686511cb40ab13c1256203a6 
>   build-support/thrift/BUILD ab19f1f68682d88f731a463c15591e45a317e760 
>   build-support/thrift/Makefile f440b610afe321af663e393a29eebda7af7bd7a8 
>   build-support/thrift/prepare_binary.sh 
> 4ad997bf039294f7940b93a76ebf014689f8f618 
>   build-support/thrift/thriftw c8debd07bc9da97fb58db795e67c9ac82cc30bc1 
>   build.gradle af119910e84c48f75f2573ababcfa287c3b986fc 
>   buildSrc/src/main/groovy/org/apache/aurora/build/ThriftPlugin.groovy 
> fc2bc9082dae2c63aa578c05dc89feb346260a67 
>   pants.ini 0671d9ab6381e5b9c324dc09a891a639cbfb2ccc 
>   src/main/python/apache/aurora/executor/BUILD 
> 486230db34a22ea5dd0f68da911c0afb1afbcac0 
> 
> 
> Diff: https://reviews.apache.org/r/62590/diff/2/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>



Re: Review Request 62590: WIP: Update to Thrift 0.10.0

2017-12-03 Thread Stephan Erb


> On Sept. 27, 2017, 1:53 a.m., Bill Farner wrote:
> > ```
> > /bin/sh: cmake: command not found
> > ```
> > 
> > But now i need to install cmake, so i'm not sure this pays off.
> 
> Bill Farner wrote:
> (this = the switch to cmake)
> 
> Stephan Erb wrote:
> Bison on MacOs is 10 years old. I assumed they have a good reason for it 
> and considered an added dependency a less risky endeavor. (Or is it just 
> negligence?)
> 
> Bill Farner wrote:
> What i meant to say - currently, i can build aurora on a stock macOS 
> machine with only a modern JDK and xcode command line utilities.  Thrift 
> 0.10.0 seems to put us in the position of choosing between:
> 
> a.) adding another step to our bootstrap routine to pre-build bison
> b.) adding bison as a build-time dependency
> c.) adding cmake as a build-time dependency
> d.) other options? (i'd like to float the idea of hosting thrift binaries 
> like how pants does)
> 
> Stephan Erb wrote:
> Giving it some thought, you are right that just requring cmake does not 
> improve the situation in any way.
> 
> I also like that we can build Aurora on a stock MacOS without much hassle 
> (or administrator rights). I think we should retain this property. This would 
> restrict us to options a) and d).
> 
> Bill Farner wrote:
> I would support (d).  We can place unofficial binaries in svn for dev 
> platforms as needed, and could support using thrift from the `PATH` as a 
> fallback.  This has the bonus of making from-scratch builds much faster.
> 
> Stephan Erb wrote:
> I saw your patch to https://github.com/morimekta/providence. Did you 
> manage to get something working with it?
> 
> Bill Farner wrote:
> I'm about 20% along.  A very large mechanical patch is needed for Aurora. 
>  The only untested piece is binary format compatibility (which providence 
> aims to achieve).  There is some incompatibility i need to investigate 
> further (trivial details like a round-tripped `null` collection turning into 
> an empty collection), but so far it looks good!  I'm proceeding with high 
> confidence, as there is also the future promise of a more approachable 
> HTTP/JSON interface via thrift IDL when using providence.
> 
> Bill Farner wrote:
> I should have clarified - i don't think we should hold back on upgrading 
> thrift.  It's not yet guaranteed that the migration to providence will be 
> successful or timely.
> 
> As for my proposed (d) above, we could store prebuilt binaries similarly 
> to how we store mesos eggs: https://svn.apache.org/repos/asf/aurora/3rdparty/

I have just pushed another patch that I have been sitting one for a few days. 
It suffers from the same pants issues as https://reviews.apache.org/r/63750/


- Stephan


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


On Dec. 3, 2017, 9:09 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62590/
> ---
> 
> (Updated Dec. 3, 2017, 9:09 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Update to Thrift 0.10.0 (https://github.com/apache/thrift/blob/master/CHANGES)
> 
> Included changes:
> 
> * Update to the latest pants version. This was necesary to make `./pants gen` 
> working. Unfortunately this breaks a few things. For details see 
> https://reviews.apache.org/r/63750/ 
> * The Java `hashcode` option has been removed as it is now the default.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 155930dc6b808fb2f573f427a4508e388ee04b5a 
>   build-support/packer/build.sh 85444125abc0c7e600a09933411e57c0d74051ac 
>   build-support/thrift/AURORA-1727.compiler.cpp.Makefile.in.patch 
> 77c966caa3d1f644241bcc2b1968bc9306c56689 
>   
> build-support/thrift/AURORA-1727.compiler.cpp.src.generate.t_java_generator.cc.patch
>  42300b43a8f72e45c96b975e5d3a6a7bd0283529 
>   build-support/thrift/AURORA-1727.lib.py.setup.py.patch 
> 11c7b13341e156f3686511cb40ab13c1256203a6 
>   build-support/thrift/BUILD ab19f1f68682d88f731a463c15591e45a317e760 
>   build-support/thrift/Makefile f440b610afe321af663e393a29eebda7af7bd7a8 
>   build-support/thrift/prepare_binary.sh 
> 4ad997bf039294f7940b93a76ebf014689f8f618 
>   build-support/thrift/thriftw c8debd07bc9da97fb58db795e67c9ac82cc30bc1 
>   build.gradle af119910e84c48f75f2573ababcfa287c3b986fc 
>   buildSrc/src/main/groovy/org/apache/aurora/build/ThriftPlugin.groovy 
> fc2bc9082dae2c63aa578c05dc89feb346260a67 
>   pants.ini 0671d9ab6381e5b9c324dc09a891a639cbfb2ccc 
>   src/main/python/apache/aurora/executor/BUILD 
> 486230db34a22ea5dd0f68da911c0afb1afbcac0 
> 
> 
> Diff: 

Re: Review Request 62590: WIP: Update to Thrift 0.10.0

2017-12-03 Thread Stephan Erb

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

(Updated Dec. 3, 2017, 9:09 p.m.)


Review request for Aurora and Bill Farner.


Changes
---

Use the Thrift binary bootstrapped by Pants also for the Java build.


Repository: aurora


Description (updated)
---

Update to Thrift 0.10.0 (https://github.com/apache/thrift/blob/master/CHANGES)

Included changes:

* Update to the latest pants version. This was necesary to make `./pants gen` 
working. Unfortunately this breaks a few things. For details see 
https://reviews.apache.org/r/63750/ 
* The Java `hashcode` option has been removed as it is now the default.


Diffs (updated)
-

  3rdparty/python/requirements.txt 155930dc6b808fb2f573f427a4508e388ee04b5a 
  build-support/packer/build.sh 85444125abc0c7e600a09933411e57c0d74051ac 
  build-support/thrift/AURORA-1727.compiler.cpp.Makefile.in.patch 
77c966caa3d1f644241bcc2b1968bc9306c56689 
  
build-support/thrift/AURORA-1727.compiler.cpp.src.generate.t_java_generator.cc.patch
 42300b43a8f72e45c96b975e5d3a6a7bd0283529 
  build-support/thrift/AURORA-1727.lib.py.setup.py.patch 
11c7b13341e156f3686511cb40ab13c1256203a6 
  build-support/thrift/BUILD ab19f1f68682d88f731a463c15591e45a317e760 
  build-support/thrift/Makefile f440b610afe321af663e393a29eebda7af7bd7a8 
  build-support/thrift/prepare_binary.sh 
4ad997bf039294f7940b93a76ebf014689f8f618 
  build-support/thrift/thriftw c8debd07bc9da97fb58db795e67c9ac82cc30bc1 
  build.gradle af119910e84c48f75f2573ababcfa287c3b986fc 
  buildSrc/src/main/groovy/org/apache/aurora/build/ThriftPlugin.groovy 
fc2bc9082dae2c63aa578c05dc89feb346260a67 
  pants.ini 0671d9ab6381e5b9c324dc09a891a639cbfb2ccc 
  src/main/python/apache/aurora/executor/BUILD 
486230db34a22ea5dd0f68da911c0afb1afbcac0 


Diff: https://reviews.apache.org/r/62590/diff/2/

Changes: https://reviews.apache.org/r/62590/diff/1-2/


Testing (updated)
---

./build-support/jenkins/build.sh


Thanks,

Stephan Erb



Re: Review Request 62590: WIP: Update to Thrift 0.10.0

2017-11-11 Thread Renan DelValle


> On Nov. 9, 2017, 6:04 p.m., Renan DelValle wrote:
> > I'm all for the upgrade to Thrift 0.10.0 but we should announce plans to 
> > upgrade ASAP to the mailing lists because it'll break every go thrift 
> > client due to https://issues.apache.org/jira/browse/THRIFT-3467 . It says 
> > it was included first in 0.11.0, but commit history shows it was actually 
> > included in 0.10.0 
> > https://github.com/apache/thrift/commit/ca714c4397ed78bd880f0dd76526e3817ecc08f0
> >  . It caused a huge amount of breakage in gorealis when I tried upgrading 
> > the dependancy for kicks so we should give everyone a fair warning.
> 
> Bill Farner wrote:
> At a cursory glance, that appears to be specific to generated go code 
> with a 0.10.0 compiler.  Is that correct?  If so, it seems like Aurora should 
> be able to update independently so long as the wire protocol is unchanged.

Yup, that is correct. The issue is more if the maintainers of those clients 
want to keep Thrift version parity. In general, we've been fine using code 
generate by a different Thrift version but you never know what may break in the 
future. I'll send out an e-mail if only to keep it for search engine reference.


- Renan


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


On Sept. 26, 2017, 1:50 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62590/
> ---
> 
> (Updated Sept. 26, 2017, 1:50 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Update to Thrift 0.10.0 (https://github.com/apache/thrift/blob/master/CHANGES)
> 
> Included changes:
> 
> * Switch to cmake rather than building with make. The cmake build is more 
> lenient with its build requirments. We would have needed to upgrade the 
> system bison on MacOS otherwise.
> * The Java `hashcode` option has been removed as it is now the default.
> 
> 
> ToDo / Questions:
> 
> * Rebuild the Aurora image to include Thrift 0.10. Until this is done, one 
> needs to manually run `sudo apt-get install flex bison cmake build-essential` 
> within the vagrant box to ensure Thrift can be bootstrapped.
> * The Gradle workaround looks strange. If it is really required we should 
> extract it into a separate patch.
> * The Pants build is complaining about the usage of `new_style` option. See 
> if we can get to work around it.
> * Do we want to maintain some patches to ensure we are just building the 
> required Thrift subset? Is this worth the effort?
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 0c04b98e791c5de1985724304f384e8dbade8c19 
>   build-support/packer/build.sh 7b7914c11e376a5f7ba7a5ee69fa0962e9bca379 
>   build-support/thrift/AURORA-1727.compiler.cpp.Makefile.in.patch 
> 77c966caa3d1f644241bcc2b1968bc9306c56689 
>   
> build-support/thrift/AURORA-1727.compiler.cpp.src.generate.t_java_generator.cc.patch
>  42300b43a8f72e45c96b975e5d3a6a7bd0283529 
>   build-support/thrift/AURORA-1727.lib.py.setup.py.patch 
> 11c7b13341e156f3686511cb40ab13c1256203a6 
>   build-support/thrift/Makefile f440b610afe321af663e393a29eebda7af7bd7a8 
>   build.gradle 851bab52fd389bc9577a6938f646bb29bae06139 
>   buildSrc/src/main/groovy/org/apache/aurora/build/ThriftPlugin.groovy 
> fc2bc9082dae2c63aa578c05dc89feb346260a67 
>   gradle.properties PRE-CREATION 
>   pants.ini f25e1d1a50f457325dbbabf327c31d760cb90339 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/Util.java 
> c28fb65010af5e3db925487929d4e0e12b4101a4 
> 
> 
> Diff: https://reviews.apache.org/r/62590/diff/1/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>



Re: Review Request 62590: WIP: Update to Thrift 0.10.0

2017-11-11 Thread Bill Farner


> On Nov. 9, 2017, 6:04 p.m., Renan DelValle wrote:
> > I'm all for the upgrade to Thrift 0.10.0 but we should announce plans to 
> > upgrade ASAP to the mailing lists because it'll break every go thrift 
> > client due to https://issues.apache.org/jira/browse/THRIFT-3467 . It says 
> > it was included first in 0.11.0, but commit history shows it was actually 
> > included in 0.10.0 
> > https://github.com/apache/thrift/commit/ca714c4397ed78bd880f0dd76526e3817ecc08f0
> >  . It caused a huge amount of breakage in gorealis when I tried upgrading 
> > the dependancy for kicks so we should give everyone a fair warning.

At a cursory glance, that appears to be specific to generated go code with a 
0.10.0 compiler.  Is that correct?  If so, it seems like Aurora should be able 
to update independently so long as the wire protocol is unchanged.


- Bill


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


On Sept. 26, 2017, 1:50 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62590/
> ---
> 
> (Updated Sept. 26, 2017, 1:50 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Update to Thrift 0.10.0 (https://github.com/apache/thrift/blob/master/CHANGES)
> 
> Included changes:
> 
> * Switch to cmake rather than building with make. The cmake build is more 
> lenient with its build requirments. We would have needed to upgrade the 
> system bison on MacOS otherwise.
> * The Java `hashcode` option has been removed as it is now the default.
> 
> 
> ToDo / Questions:
> 
> * Rebuild the Aurora image to include Thrift 0.10. Until this is done, one 
> needs to manually run `sudo apt-get install flex bison cmake build-essential` 
> within the vagrant box to ensure Thrift can be bootstrapped.
> * The Gradle workaround looks strange. If it is really required we should 
> extract it into a separate patch.
> * The Pants build is complaining about the usage of `new_style` option. See 
> if we can get to work around it.
> * Do we want to maintain some patches to ensure we are just building the 
> required Thrift subset? Is this worth the effort?
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 0c04b98e791c5de1985724304f384e8dbade8c19 
>   build-support/packer/build.sh 7b7914c11e376a5f7ba7a5ee69fa0962e9bca379 
>   build-support/thrift/AURORA-1727.compiler.cpp.Makefile.in.patch 
> 77c966caa3d1f644241bcc2b1968bc9306c56689 
>   
> build-support/thrift/AURORA-1727.compiler.cpp.src.generate.t_java_generator.cc.patch
>  42300b43a8f72e45c96b975e5d3a6a7bd0283529 
>   build-support/thrift/AURORA-1727.lib.py.setup.py.patch 
> 11c7b13341e156f3686511cb40ab13c1256203a6 
>   build-support/thrift/Makefile f440b610afe321af663e393a29eebda7af7bd7a8 
>   build.gradle 851bab52fd389bc9577a6938f646bb29bae06139 
>   buildSrc/src/main/groovy/org/apache/aurora/build/ThriftPlugin.groovy 
> fc2bc9082dae2c63aa578c05dc89feb346260a67 
>   gradle.properties PRE-CREATION 
>   pants.ini f25e1d1a50f457325dbbabf327c31d760cb90339 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/Util.java 
> c28fb65010af5e3db925487929d4e0e12b4101a4 
> 
> 
> Diff: https://reviews.apache.org/r/62590/diff/1/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>



Re: Review Request 62590: WIP: Update to Thrift 0.10.0

2017-11-09 Thread Renan DelValle

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



I'm all for the upgrade to Thrift 0.10.0 but we should announce plans to 
upgrade ASAP to the mailing lists because it'll break every go thrift client 
due to https://issues.apache.org/jira/browse/THRIFT-3467 . It says it was 
included first in 0.11.0, but commit history shows it was actually included in 
0.10.0 
https://github.com/apache/thrift/commit/ca714c4397ed78bd880f0dd76526e3817ecc08f0
 . It caused a huge amount of breakage in gorealis when I tried upgrading the 
dependancy for kicks so we should give everyone a fair warning.

- Renan DelValle


On Sept. 26, 2017, 1:50 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62590/
> ---
> 
> (Updated Sept. 26, 2017, 1:50 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Update to Thrift 0.10.0 (https://github.com/apache/thrift/blob/master/CHANGES)
> 
> Included changes:
> 
> * Switch to cmake rather than building with make. The cmake build is more 
> lenient with its build requirments. We would have needed to upgrade the 
> system bison on MacOS otherwise.
> * The Java `hashcode` option has been removed as it is now the default.
> 
> 
> ToDo / Questions:
> 
> * Rebuild the Aurora image to include Thrift 0.10. Until this is done, one 
> needs to manually run `sudo apt-get install flex bison cmake build-essential` 
> within the vagrant box to ensure Thrift can be bootstrapped.
> * The Gradle workaround looks strange. If it is really required we should 
> extract it into a separate patch.
> * The Pants build is complaining about the usage of `new_style` option. See 
> if we can get to work around it.
> * Do we want to maintain some patches to ensure we are just building the 
> required Thrift subset? Is this worth the effort?
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 0c04b98e791c5de1985724304f384e8dbade8c19 
>   build-support/packer/build.sh 7b7914c11e376a5f7ba7a5ee69fa0962e9bca379 
>   build-support/thrift/AURORA-1727.compiler.cpp.Makefile.in.patch 
> 77c966caa3d1f644241bcc2b1968bc9306c56689 
>   
> build-support/thrift/AURORA-1727.compiler.cpp.src.generate.t_java_generator.cc.patch
>  42300b43a8f72e45c96b975e5d3a6a7bd0283529 
>   build-support/thrift/AURORA-1727.lib.py.setup.py.patch 
> 11c7b13341e156f3686511cb40ab13c1256203a6 
>   build-support/thrift/Makefile f440b610afe321af663e393a29eebda7af7bd7a8 
>   build.gradle 851bab52fd389bc9577a6938f646bb29bae06139 
>   buildSrc/src/main/groovy/org/apache/aurora/build/ThriftPlugin.groovy 
> fc2bc9082dae2c63aa578c05dc89feb346260a67 
>   gradle.properties PRE-CREATION 
>   pants.ini f25e1d1a50f457325dbbabf327c31d760cb90339 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/Util.java 
> c28fb65010af5e3db925487929d4e0e12b4101a4 
> 
> 
> Diff: https://reviews.apache.org/r/62590/diff/1/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>



Re: Review Request 62590: WIP: Update to Thrift 0.10.0

2017-11-09 Thread Bill Farner


> On Sept. 26, 2017, 4:53 p.m., Bill Farner wrote:
> > ```
> > /bin/sh: cmake: command not found
> > ```
> > 
> > But now i need to install cmake, so i'm not sure this pays off.
> 
> Bill Farner wrote:
> (this = the switch to cmake)
> 
> Stephan Erb wrote:
> Bison on MacOs is 10 years old. I assumed they have a good reason for it 
> and considered an added dependency a less risky endeavor. (Or is it just 
> negligence?)
> 
> Bill Farner wrote:
> What i meant to say - currently, i can build aurora on a stock macOS 
> machine with only a modern JDK and xcode command line utilities.  Thrift 
> 0.10.0 seems to put us in the position of choosing between:
> 
> a.) adding another step to our bootstrap routine to pre-build bison
> b.) adding bison as a build-time dependency
> c.) adding cmake as a build-time dependency
> d.) other options? (i'd like to float the idea of hosting thrift binaries 
> like how pants does)
> 
> Stephan Erb wrote:
> Giving it some thought, you are right that just requring cmake does not 
> improve the situation in any way.
> 
> I also like that we can build Aurora on a stock MacOS without much hassle 
> (or administrator rights). I think we should retain this property. This would 
> restrict us to options a) and d).
> 
> Bill Farner wrote:
> I would support (d).  We can place unofficial binaries in svn for dev 
> platforms as needed, and could support using thrift from the `PATH` as a 
> fallback.  This has the bonus of making from-scratch builds much faster.
> 
> Stephan Erb wrote:
> I saw your patch to https://github.com/morimekta/providence. Did you 
> manage to get something working with it?
> 
> Bill Farner wrote:
> I'm about 20% along.  A very large mechanical patch is needed for Aurora. 
>  The only untested piece is binary format compatibility (which providence 
> aims to achieve).  There is some incompatibility i need to investigate 
> further (trivial details like a round-tripped `null` collection turning into 
> an empty collection), but so far it looks good!  I'm proceeding with high 
> confidence, as there is also the future promise of a more approachable 
> HTTP/JSON interface via thrift IDL when using providence.

I should have clarified - i don't think we should hold back on upgrading 
thrift.  It's not yet guaranteed that the migration to providence will be 
successful or timely.

As for my proposed (d) above, we could store prebuilt binaries similarly to how 
we store mesos eggs: https://svn.apache.org/repos/asf/aurora/3rdparty/


- Bill


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


On Sept. 26, 2017, 1:50 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62590/
> ---
> 
> (Updated Sept. 26, 2017, 1:50 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Update to Thrift 0.10.0 (https://github.com/apache/thrift/blob/master/CHANGES)
> 
> Included changes:
> 
> * Switch to cmake rather than building with make. The cmake build is more 
> lenient with its build requirments. We would have needed to upgrade the 
> system bison on MacOS otherwise.
> * The Java `hashcode` option has been removed as it is now the default.
> 
> 
> ToDo / Questions:
> 
> * Rebuild the Aurora image to include Thrift 0.10. Until this is done, one 
> needs to manually run `sudo apt-get install flex bison cmake build-essential` 
> within the vagrant box to ensure Thrift can be bootstrapped.
> * The Gradle workaround looks strange. If it is really required we should 
> extract it into a separate patch.
> * The Pants build is complaining about the usage of `new_style` option. See 
> if we can get to work around it.
> * Do we want to maintain some patches to ensure we are just building the 
> required Thrift subset? Is this worth the effort?
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 0c04b98e791c5de1985724304f384e8dbade8c19 
>   build-support/packer/build.sh 7b7914c11e376a5f7ba7a5ee69fa0962e9bca379 
>   build-support/thrift/AURORA-1727.compiler.cpp.Makefile.in.patch 
> 77c966caa3d1f644241bcc2b1968bc9306c56689 
>   
> build-support/thrift/AURORA-1727.compiler.cpp.src.generate.t_java_generator.cc.patch
>  42300b43a8f72e45c96b975e5d3a6a7bd0283529 
>   build-support/thrift/AURORA-1727.lib.py.setup.py.patch 
> 11c7b13341e156f3686511cb40ab13c1256203a6 
>   build-support/thrift/Makefile f440b610afe321af663e393a29eebda7af7bd7a8 
>   build.gradle 851bab52fd389bc9577a6938f646bb29bae06139 
>   buildSrc/src/main/groovy/org/apache/aurora/build/ThriftPlugin.groovy 
> fc2bc9082dae2c63aa578c05dc89feb346260a67 

Re: Review Request 62590: WIP: Update to Thrift 0.10.0

2017-10-25 Thread Bill Farner


> On Sept. 26, 2017, 4:53 p.m., Bill Farner wrote:
> > ```
> > /bin/sh: cmake: command not found
> > ```
> > 
> > But now i need to install cmake, so i'm not sure this pays off.
> 
> Bill Farner wrote:
> (this = the switch to cmake)
> 
> Stephan Erb wrote:
> Bison on MacOs is 10 years old. I assumed they have a good reason for it 
> and considered an added dependency a less risky endeavor. (Or is it just 
> negligence?)
> 
> Bill Farner wrote:
> What i meant to say - currently, i can build aurora on a stock macOS 
> machine with only a modern JDK and xcode command line utilities.  Thrift 
> 0.10.0 seems to put us in the position of choosing between:
> 
> a.) adding another step to our bootstrap routine to pre-build bison
> b.) adding bison as a build-time dependency
> c.) adding cmake as a build-time dependency
> d.) other options? (i'd like to float the idea of hosting thrift binaries 
> like how pants does)
> 
> Stephan Erb wrote:
> Giving it some thought, you are right that just requring cmake does not 
> improve the situation in any way.
> 
> I also like that we can build Aurora on a stock MacOS without much hassle 
> (or administrator rights). I think we should retain this property. This would 
> restrict us to options a) and d).
> 
> Bill Farner wrote:
> I would support (d).  We can place unofficial binaries in svn for dev 
> platforms as needed, and could support using thrift from the `PATH` as a 
> fallback.  This has the bonus of making from-scratch builds much faster.
> 
> Stephan Erb wrote:
> I saw your patch to https://github.com/morimekta/providence. Did you 
> manage to get something working with it?

I'm about 20% along.  A very large mechanical patch is needed for Aurora.  The 
only untested piece is binary format compatibility (which providence aims to 
achieve).  There is some incompatibility i need to investigate further (trivial 
details like a round-tripped `null` collection turning into an empty 
collection), but so far it looks good!  I'm proceeding with high confidence, as 
there is also the future promise of a more approachable HTTP/JSON interface via 
thrift IDL when using providence.


- Bill


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


On Sept. 26, 2017, 1:50 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62590/
> ---
> 
> (Updated Sept. 26, 2017, 1:50 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Update to Thrift 0.10.0 (https://github.com/apache/thrift/blob/master/CHANGES)
> 
> Included changes:
> 
> * Switch to cmake rather than building with make. The cmake build is more 
> lenient with its build requirments. We would have needed to upgrade the 
> system bison on MacOS otherwise.
> * The Java `hashcode` option has been removed as it is now the default.
> 
> 
> ToDo / Questions:
> 
> * Rebuild the Aurora image to include Thrift 0.10. Until this is done, one 
> needs to manually run `sudo apt-get install flex bison cmake build-essential` 
> within the vagrant box to ensure Thrift can be bootstrapped.
> * The Gradle workaround looks strange. If it is really required we should 
> extract it into a separate patch.
> * The Pants build is complaining about the usage of `new_style` option. See 
> if we can get to work around it.
> * Do we want to maintain some patches to ensure we are just building the 
> required Thrift subset? Is this worth the effort?
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 0c04b98e791c5de1985724304f384e8dbade8c19 
>   build-support/packer/build.sh 7b7914c11e376a5f7ba7a5ee69fa0962e9bca379 
>   build-support/thrift/AURORA-1727.compiler.cpp.Makefile.in.patch 
> 77c966caa3d1f644241bcc2b1968bc9306c56689 
>   
> build-support/thrift/AURORA-1727.compiler.cpp.src.generate.t_java_generator.cc.patch
>  42300b43a8f72e45c96b975e5d3a6a7bd0283529 
>   build-support/thrift/AURORA-1727.lib.py.setup.py.patch 
> 11c7b13341e156f3686511cb40ab13c1256203a6 
>   build-support/thrift/Makefile f440b610afe321af663e393a29eebda7af7bd7a8 
>   build.gradle 851bab52fd389bc9577a6938f646bb29bae06139 
>   buildSrc/src/main/groovy/org/apache/aurora/build/ThriftPlugin.groovy 
> fc2bc9082dae2c63aa578c05dc89feb346260a67 
>   gradle.properties PRE-CREATION 
>   pants.ini f25e1d1a50f457325dbbabf327c31d760cb90339 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/Util.java 
> c28fb65010af5e3db925487929d4e0e12b4101a4 
> 
> 
> Diff: https://reviews.apache.org/r/62590/diff/1/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 

Re: Review Request 62590: WIP: Update to Thrift 0.10.0

2017-10-25 Thread Stephan Erb


> On Sept. 27, 2017, 1:53 a.m., Bill Farner wrote:
> > ```
> > /bin/sh: cmake: command not found
> > ```
> > 
> > But now i need to install cmake, so i'm not sure this pays off.
> 
> Bill Farner wrote:
> (this = the switch to cmake)
> 
> Stephan Erb wrote:
> Bison on MacOs is 10 years old. I assumed they have a good reason for it 
> and considered an added dependency a less risky endeavor. (Or is it just 
> negligence?)
> 
> Bill Farner wrote:
> What i meant to say - currently, i can build aurora on a stock macOS 
> machine with only a modern JDK and xcode command line utilities.  Thrift 
> 0.10.0 seems to put us in the position of choosing between:
> 
> a.) adding another step to our bootstrap routine to pre-build bison
> b.) adding bison as a build-time dependency
> c.) adding cmake as a build-time dependency
> d.) other options? (i'd like to float the idea of hosting thrift binaries 
> like how pants does)
> 
> Stephan Erb wrote:
> Giving it some thought, you are right that just requring cmake does not 
> improve the situation in any way.
> 
> I also like that we can build Aurora on a stock MacOS without much hassle 
> (or administrator rights). I think we should retain this property. This would 
> restrict us to options a) and d).
> 
> Bill Farner wrote:
> I would support (d).  We can place unofficial binaries in svn for dev 
> platforms as needed, and could support using thrift from the `PATH` as a 
> fallback.  This has the bonus of making from-scratch builds much faster.

I saw your patch to https://github.com/morimekta/providence. Did you manage to 
get something working with it?


- Stephan


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


On Sept. 26, 2017, 10:50 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62590/
> ---
> 
> (Updated Sept. 26, 2017, 10:50 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Update to Thrift 0.10.0 (https://github.com/apache/thrift/blob/master/CHANGES)
> 
> Included changes:
> 
> * Switch to cmake rather than building with make. The cmake build is more 
> lenient with its build requirments. We would have needed to upgrade the 
> system bison on MacOS otherwise.
> * The Java `hashcode` option has been removed as it is now the default.
> 
> 
> ToDo / Questions:
> 
> * Rebuild the Aurora image to include Thrift 0.10. Until this is done, one 
> needs to manually run `sudo apt-get install flex bison cmake build-essential` 
> within the vagrant box to ensure Thrift can be bootstrapped.
> * The Gradle workaround looks strange. If it is really required we should 
> extract it into a separate patch.
> * The Pants build is complaining about the usage of `new_style` option. See 
> if we can get to work around it.
> * Do we want to maintain some patches to ensure we are just building the 
> required Thrift subset? Is this worth the effort?
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 0c04b98e791c5de1985724304f384e8dbade8c19 
>   build-support/packer/build.sh 7b7914c11e376a5f7ba7a5ee69fa0962e9bca379 
>   build-support/thrift/AURORA-1727.compiler.cpp.Makefile.in.patch 
> 77c966caa3d1f644241bcc2b1968bc9306c56689 
>   
> build-support/thrift/AURORA-1727.compiler.cpp.src.generate.t_java_generator.cc.patch
>  42300b43a8f72e45c96b975e5d3a6a7bd0283529 
>   build-support/thrift/AURORA-1727.lib.py.setup.py.patch 
> 11c7b13341e156f3686511cb40ab13c1256203a6 
>   build-support/thrift/Makefile f440b610afe321af663e393a29eebda7af7bd7a8 
>   build.gradle 851bab52fd389bc9577a6938f646bb29bae06139 
>   buildSrc/src/main/groovy/org/apache/aurora/build/ThriftPlugin.groovy 
> fc2bc9082dae2c63aa578c05dc89feb346260a67 
>   gradle.properties PRE-CREATION 
>   pants.ini f25e1d1a50f457325dbbabf327c31d760cb90339 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/Util.java 
> c28fb65010af5e3db925487929d4e0e12b4101a4 
> 
> 
> Diff: https://reviews.apache.org/r/62590/diff/1/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>



Re: Review Request 62590: WIP: Update to Thrift 0.10.0

2017-10-03 Thread Bill Farner


> On Sept. 26, 2017, 4:53 p.m., Bill Farner wrote:
> > ```
> > /bin/sh: cmake: command not found
> > ```
> > 
> > But now i need to install cmake, so i'm not sure this pays off.
> 
> Bill Farner wrote:
> (this = the switch to cmake)
> 
> Stephan Erb wrote:
> Bison on MacOs is 10 years old. I assumed they have a good reason for it 
> and considered an added dependency a less risky endeavor. (Or is it just 
> negligence?)
> 
> Bill Farner wrote:
> What i meant to say - currently, i can build aurora on a stock macOS 
> machine with only a modern JDK and xcode command line utilities.  Thrift 
> 0.10.0 seems to put us in the position of choosing between:
> 
> a.) adding another step to our bootstrap routine to pre-build bison
> b.) adding bison as a build-time dependency
> c.) adding cmake as a build-time dependency
> d.) other options? (i'd like to float the idea of hosting thrift binaries 
> like how pants does)
> 
> Stephan Erb wrote:
> Giving it some thought, you are right that just requring cmake does not 
> improve the situation in any way.
> 
> I also like that we can build Aurora on a stock MacOS without much hassle 
> (or administrator rights). I think we should retain this property. This would 
> restrict us to options a) and d).

I would support (d).  We can place unofficial binaries in svn for dev platforms 
as needed, and could support using thrift from the `PATH` as a fallback.  This 
has the bonus of making from-scratch builds much faster.


- Bill


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


On Sept. 26, 2017, 1:50 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62590/
> ---
> 
> (Updated Sept. 26, 2017, 1:50 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Update to Thrift 0.10.0 (https://github.com/apache/thrift/blob/master/CHANGES)
> 
> Included changes:
> 
> * Switch to cmake rather than building with make. The cmake build is more 
> lenient with its build requirments. We would have needed to upgrade the 
> system bison on MacOS otherwise.
> * The Java `hashcode` option has been removed as it is now the default.
> 
> 
> ToDo / Questions:
> 
> * Rebuild the Aurora image to include Thrift 0.10. Until this is done, one 
> needs to manually run `sudo apt-get install flex bison cmake build-essential` 
> within the vagrant box to ensure Thrift can be bootstrapped.
> * The Gradle workaround looks strange. If it is really required we should 
> extract it into a separate patch.
> * The Pants build is complaining about the usage of `new_style` option. See 
> if we can get to work around it.
> * Do we want to maintain some patches to ensure we are just building the 
> required Thrift subset? Is this worth the effort?
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 0c04b98e791c5de1985724304f384e8dbade8c19 
>   build-support/packer/build.sh 7b7914c11e376a5f7ba7a5ee69fa0962e9bca379 
>   build-support/thrift/AURORA-1727.compiler.cpp.Makefile.in.patch 
> 77c966caa3d1f644241bcc2b1968bc9306c56689 
>   
> build-support/thrift/AURORA-1727.compiler.cpp.src.generate.t_java_generator.cc.patch
>  42300b43a8f72e45c96b975e5d3a6a7bd0283529 
>   build-support/thrift/AURORA-1727.lib.py.setup.py.patch 
> 11c7b13341e156f3686511cb40ab13c1256203a6 
>   build-support/thrift/Makefile f440b610afe321af663e393a29eebda7af7bd7a8 
>   build.gradle 851bab52fd389bc9577a6938f646bb29bae06139 
>   buildSrc/src/main/groovy/org/apache/aurora/build/ThriftPlugin.groovy 
> fc2bc9082dae2c63aa578c05dc89feb346260a67 
>   gradle.properties PRE-CREATION 
>   pants.ini f25e1d1a50f457325dbbabf327c31d760cb90339 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/Util.java 
> c28fb65010af5e3db925487929d4e0e12b4101a4 
> 
> 
> Diff: https://reviews.apache.org/r/62590/diff/1/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>



Re: Review Request 62590: WIP: Update to Thrift 0.10.0

2017-10-03 Thread Stephan Erb


> On Sept. 27, 2017, 1:53 a.m., Bill Farner wrote:
> > ```
> > /bin/sh: cmake: command not found
> > ```
> > 
> > But now i need to install cmake, so i'm not sure this pays off.
> 
> Bill Farner wrote:
> (this = the switch to cmake)
> 
> Stephan Erb wrote:
> Bison on MacOs is 10 years old. I assumed they have a good reason for it 
> and considered an added dependency a less risky endeavor. (Or is it just 
> negligence?)
> 
> Bill Farner wrote:
> What i meant to say - currently, i can build aurora on a stock macOS 
> machine with only a modern JDK and xcode command line utilities.  Thrift 
> 0.10.0 seems to put us in the position of choosing between:
> 
> a.) adding another step to our bootstrap routine to pre-build bison
> b.) adding bison as a build-time dependency
> c.) adding cmake as a build-time dependency
> d.) other options? (i'd like to float the idea of hosting thrift binaries 
> like how pants does)

Giving it some thought, you are right that just requring cmake does not improve 
the situation in any way.

I also like that we can build Aurora on a stock MacOS without much hassle (or 
administrator rights). I think we should retain this property. This would 
restrict us to options a) and d).


- Stephan


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


On Sept. 26, 2017, 10:50 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62590/
> ---
> 
> (Updated Sept. 26, 2017, 10:50 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Update to Thrift 0.10.0 (https://github.com/apache/thrift/blob/master/CHANGES)
> 
> Included changes:
> 
> * Switch to cmake rather than building with make. The cmake build is more 
> lenient with its build requirments. We would have needed to upgrade the 
> system bison on MacOS otherwise.
> * The Java `hashcode` option has been removed as it is now the default.
> 
> 
> ToDo / Questions:
> 
> * Rebuild the Aurora image to include Thrift 0.10. Until this is done, one 
> needs to manually run `sudo apt-get install flex bison cmake build-essential` 
> within the vagrant box to ensure Thrift can be bootstrapped.
> * The Gradle workaround looks strange. If it is really required we should 
> extract it into a separate patch.
> * The Pants build is complaining about the usage of `new_style` option. See 
> if we can get to work around it.
> * Do we want to maintain some patches to ensure we are just building the 
> required Thrift subset? Is this worth the effort?
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 0c04b98e791c5de1985724304f384e8dbade8c19 
>   build-support/packer/build.sh 7b7914c11e376a5f7ba7a5ee69fa0962e9bca379 
>   build-support/thrift/AURORA-1727.compiler.cpp.Makefile.in.patch 
> 77c966caa3d1f644241bcc2b1968bc9306c56689 
>   
> build-support/thrift/AURORA-1727.compiler.cpp.src.generate.t_java_generator.cc.patch
>  42300b43a8f72e45c96b975e5d3a6a7bd0283529 
>   build-support/thrift/AURORA-1727.lib.py.setup.py.patch 
> 11c7b13341e156f3686511cb40ab13c1256203a6 
>   build-support/thrift/Makefile f440b610afe321af663e393a29eebda7af7bd7a8 
>   build.gradle 851bab52fd389bc9577a6938f646bb29bae06139 
>   buildSrc/src/main/groovy/org/apache/aurora/build/ThriftPlugin.groovy 
> fc2bc9082dae2c63aa578c05dc89feb346260a67 
>   gradle.properties PRE-CREATION 
>   pants.ini f25e1d1a50f457325dbbabf327c31d760cb90339 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/Util.java 
> c28fb65010af5e3db925487929d4e0e12b4101a4 
> 
> 
> Diff: https://reviews.apache.org/r/62590/diff/1/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>



Re: Review Request 62590: WIP: Update to Thrift 0.10.0

2017-09-27 Thread Bill Farner


> On Sept. 26, 2017, 4:53 p.m., Bill Farner wrote:
> > ```
> > /bin/sh: cmake: command not found
> > ```
> > 
> > But now i need to install cmake, so i'm not sure this pays off.
> 
> Bill Farner wrote:
> (this = the switch to cmake)
> 
> Stephan Erb wrote:
> Bison on MacOs is 10 years old. I assumed they have a good reason for it 
> and considered an added dependency a less risky endeavor. (Or is it just 
> negligence?)

What i meant to say - currently, i can build aurora on a stock macOS machine 
with only a modern JDK and xcode command line utilities.  Thrift 0.10.0 seems 
to put us in the position of choosing between:

a.) adding another step to our bootstrap routine to pre-build bison
b.) adding bison as a build-time dependency
c.) adding cmake as a build-time dependency
d.) other options? (i'd like to float the idea of hosting thrift binaries like 
how pants does)


- Bill


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


On Sept. 26, 2017, 1:50 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62590/
> ---
> 
> (Updated Sept. 26, 2017, 1:50 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Update to Thrift 0.10.0 (https://github.com/apache/thrift/blob/master/CHANGES)
> 
> Included changes:
> 
> * Switch to cmake rather than building with make. The cmake build is more 
> lenient with its build requirments. We would have needed to upgrade the 
> system bison on MacOS otherwise.
> * The Java `hashcode` option has been removed as it is now the default.
> 
> 
> ToDo / Questions:
> 
> * Rebuild the Aurora image to include Thrift 0.10. Until this is done, one 
> needs to manually run `sudo apt-get install flex bison cmake build-essential` 
> within the vagrant box to ensure Thrift can be bootstrapped.
> * The Gradle workaround looks strange. If it is really required we should 
> extract it into a separate patch.
> * The Pants build is complaining about the usage of `new_style` option. See 
> if we can get to work around it.
> * Do we want to maintain some patches to ensure we are just building the 
> required Thrift subset? Is this worth the effort?
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 0c04b98e791c5de1985724304f384e8dbade8c19 
>   build-support/packer/build.sh 7b7914c11e376a5f7ba7a5ee69fa0962e9bca379 
>   build-support/thrift/AURORA-1727.compiler.cpp.Makefile.in.patch 
> 77c966caa3d1f644241bcc2b1968bc9306c56689 
>   
> build-support/thrift/AURORA-1727.compiler.cpp.src.generate.t_java_generator.cc.patch
>  42300b43a8f72e45c96b975e5d3a6a7bd0283529 
>   build-support/thrift/AURORA-1727.lib.py.setup.py.patch 
> 11c7b13341e156f3686511cb40ab13c1256203a6 
>   build-support/thrift/Makefile f440b610afe321af663e393a29eebda7af7bd7a8 
>   build.gradle 851bab52fd389bc9577a6938f646bb29bae06139 
>   buildSrc/src/main/groovy/org/apache/aurora/build/ThriftPlugin.groovy 
> fc2bc9082dae2c63aa578c05dc89feb346260a67 
>   gradle.properties PRE-CREATION 
>   pants.ini f25e1d1a50f457325dbbabf327c31d760cb90339 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/Util.java 
> c28fb65010af5e3db925487929d4e0e12b4101a4 
> 
> 
> Diff: https://reviews.apache.org/r/62590/diff/1/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>



Re: Review Request 62590: WIP: Update to Thrift 0.10.0

2017-09-27 Thread Stephan Erb


> On Sept. 27, 2017, 1:53 a.m., Bill Farner wrote:
> > ```
> > /bin/sh: cmake: command not found
> > ```
> > 
> > But now i need to install cmake, so i'm not sure this pays off.
> 
> Bill Farner wrote:
> (this = the switch to cmake)

Bison on MacOs is 10 years old. I assumed they have a good reason for it and 
considered an added dependency a less risky endeavor. (Or is it just 
negligence?)


- Stephan


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


On Sept. 26, 2017, 10:50 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62590/
> ---
> 
> (Updated Sept. 26, 2017, 10:50 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Update to Thrift 0.10.0 (https://github.com/apache/thrift/blob/master/CHANGES)
> 
> Included changes:
> 
> * Switch to cmake rather than building with make. The cmake build is more 
> lenient with its build requirments. We would have needed to upgrade the 
> system bison on MacOS otherwise.
> * The Java `hashcode` option has been removed as it is now the default.
> 
> 
> ToDo / Questions:
> 
> * Rebuild the Aurora image to include Thrift 0.10. Until this is done, one 
> needs to manually run `sudo apt-get install flex bison cmake build-essential` 
> within the vagrant box to ensure Thrift can be bootstrapped.
> * The Gradle workaround looks strange. If it is really required we should 
> extract it into a separate patch.
> * The Pants build is complaining about the usage of `new_style` option. See 
> if we can get to work around it.
> * Do we want to maintain some patches to ensure we are just building the 
> required Thrift subset? Is this worth the effort?
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 0c04b98e791c5de1985724304f384e8dbade8c19 
>   build-support/packer/build.sh 7b7914c11e376a5f7ba7a5ee69fa0962e9bca379 
>   build-support/thrift/AURORA-1727.compiler.cpp.Makefile.in.patch 
> 77c966caa3d1f644241bcc2b1968bc9306c56689 
>   
> build-support/thrift/AURORA-1727.compiler.cpp.src.generate.t_java_generator.cc.patch
>  42300b43a8f72e45c96b975e5d3a6a7bd0283529 
>   build-support/thrift/AURORA-1727.lib.py.setup.py.patch 
> 11c7b13341e156f3686511cb40ab13c1256203a6 
>   build-support/thrift/Makefile f440b610afe321af663e393a29eebda7af7bd7a8 
>   build.gradle 851bab52fd389bc9577a6938f646bb29bae06139 
>   buildSrc/src/main/groovy/org/apache/aurora/build/ThriftPlugin.groovy 
> fc2bc9082dae2c63aa578c05dc89feb346260a67 
>   gradle.properties PRE-CREATION 
>   pants.ini f25e1d1a50f457325dbbabf327c31d760cb90339 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/Util.java 
> c28fb65010af5e3db925487929d4e0e12b4101a4 
> 
> 
> Diff: https://reviews.apache.org/r/62590/diff/1/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>



Re: Review Request 62590: WIP: Update to Thrift 0.10.0

2017-09-26 Thread Bill Farner

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



```
/bin/sh: cmake: command not found
```

But now i need to install cmake, so i'm not sure this pays off.

- Bill Farner


On Sept. 26, 2017, 1:50 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62590/
> ---
> 
> (Updated Sept. 26, 2017, 1:50 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Update to Thrift 0.10.0 (https://github.com/apache/thrift/blob/master/CHANGES)
> 
> Included changes:
> 
> * Switch to cmake rather than building with make. The cmake build is more 
> lenient with its build requirments. We would have needed to upgrade the 
> system bison on MacOS otherwise.
> * The Java `hashcode` option has been removed as it is now the default.
> 
> 
> ToDo / Questions:
> 
> * Rebuild the Aurora image to include Thrift 0.10. Until this is done, one 
> needs to manually run `sudo apt-get install flex bison cmake build-essential` 
> within the vagrant box to ensure Thrift can be bootstrapped.
> * The Gradle workaround looks strange. If it is really required we should 
> extract it into a separate patch.
> * The Pants build is complaining about the usage of `new_style` option. See 
> if we can get to work around it.
> * Do we want to maintain some patches to ensure we are just building the 
> required Thrift subset? Is this worth the effort?
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 0c04b98e791c5de1985724304f384e8dbade8c19 
>   build-support/packer/build.sh 7b7914c11e376a5f7ba7a5ee69fa0962e9bca379 
>   build-support/thrift/AURORA-1727.compiler.cpp.Makefile.in.patch 
> 77c966caa3d1f644241bcc2b1968bc9306c56689 
>   
> build-support/thrift/AURORA-1727.compiler.cpp.src.generate.t_java_generator.cc.patch
>  42300b43a8f72e45c96b975e5d3a6a7bd0283529 
>   build-support/thrift/AURORA-1727.lib.py.setup.py.patch 
> 11c7b13341e156f3686511cb40ab13c1256203a6 
>   build-support/thrift/Makefile f440b610afe321af663e393a29eebda7af7bd7a8 
>   build.gradle 851bab52fd389bc9577a6938f646bb29bae06139 
>   buildSrc/src/main/groovy/org/apache/aurora/build/ThriftPlugin.groovy 
> fc2bc9082dae2c63aa578c05dc89feb346260a67 
>   gradle.properties PRE-CREATION 
>   pants.ini f25e1d1a50f457325dbbabf327c31d760cb90339 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/Util.java 
> c28fb65010af5e3db925487929d4e0e12b4101a4 
> 
> 
> Diff: https://reviews.apache.org/r/62590/diff/1/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>



Re: Review Request 62590: WIP: Update to Thrift 0.10.0

2017-09-26 Thread Bill Farner


> On Sept. 26, 2017, 4:53 p.m., Bill Farner wrote:
> > ```
> > /bin/sh: cmake: command not found
> > ```
> > 
> > But now i need to install cmake, so i'm not sure this pays off.

(this = the switch to cmake)


- Bill


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


On Sept. 26, 2017, 1:50 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62590/
> ---
> 
> (Updated Sept. 26, 2017, 1:50 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Update to Thrift 0.10.0 (https://github.com/apache/thrift/blob/master/CHANGES)
> 
> Included changes:
> 
> * Switch to cmake rather than building with make. The cmake build is more 
> lenient with its build requirments. We would have needed to upgrade the 
> system bison on MacOS otherwise.
> * The Java `hashcode` option has been removed as it is now the default.
> 
> 
> ToDo / Questions:
> 
> * Rebuild the Aurora image to include Thrift 0.10. Until this is done, one 
> needs to manually run `sudo apt-get install flex bison cmake build-essential` 
> within the vagrant box to ensure Thrift can be bootstrapped.
> * The Gradle workaround looks strange. If it is really required we should 
> extract it into a separate patch.
> * The Pants build is complaining about the usage of `new_style` option. See 
> if we can get to work around it.
> * Do we want to maintain some patches to ensure we are just building the 
> required Thrift subset? Is this worth the effort?
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 0c04b98e791c5de1985724304f384e8dbade8c19 
>   build-support/packer/build.sh 7b7914c11e376a5f7ba7a5ee69fa0962e9bca379 
>   build-support/thrift/AURORA-1727.compiler.cpp.Makefile.in.patch 
> 77c966caa3d1f644241bcc2b1968bc9306c56689 
>   
> build-support/thrift/AURORA-1727.compiler.cpp.src.generate.t_java_generator.cc.patch
>  42300b43a8f72e45c96b975e5d3a6a7bd0283529 
>   build-support/thrift/AURORA-1727.lib.py.setup.py.patch 
> 11c7b13341e156f3686511cb40ab13c1256203a6 
>   build-support/thrift/Makefile f440b610afe321af663e393a29eebda7af7bd7a8 
>   build.gradle 851bab52fd389bc9577a6938f646bb29bae06139 
>   buildSrc/src/main/groovy/org/apache/aurora/build/ThriftPlugin.groovy 
> fc2bc9082dae2c63aa578c05dc89feb346260a67 
>   gradle.properties PRE-CREATION 
>   pants.ini f25e1d1a50f457325dbbabf327c31d760cb90339 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/Util.java 
> c28fb65010af5e3db925487929d4e0e12b4101a4 
> 
> 
> Diff: https://reviews.apache.org/r/62590/diff/1/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>



Re: Review Request 62590: WIP: Update to Thrift 0.10.0

2017-09-26 Thread Aurora ReviewBot

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



Master (0606369) is red with this patch.
  ./build-support/jenkins/build.sh

--   Build Thrift libraries: ON
--  Language libraries:
--   Build C++ library:  ON
--- Boost headers missing
--   Build C (GLib) library: ON
--   Build Java library: ON
--   Build Python library:   ON
--   Build Haskell library:  ON
--  Library features:
--   Build shared libraries: ON
--   Build static libraries: ON
--   Build with ZLIB support:ON
--   Build with libevent support:ON
--   Build with Qt4 support: ON
--   Build with Qt5 support: OFF
--   Build with OpenSSL support: ON
--   Build with Boost thread support:OFF
--   Build with C++ std::thread support: OFF
--   Build with Boost static link library:   OFF
-- --
-- Configuring incomplete, errors occurred!
See also 
"/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift/thrift-0.10.0/cmake-build/CMakeFiles/CMakeOutput.log".
See also 
"/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift/thrift-0.10.0/cmake-build/CMakeFiles/CMakeError.log".
make: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift'
make: *** [thrift-0.10.0/compiler/cpp/thrift] Error 1
:api:generateThriftJava FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':api:generateThriftJava'.
> Process 'command 
> '/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift/thriftw''
>  finished with non-zero exit value 2

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.

* Get more help at https://help.gradle.org

BUILD FAILED in 48s
7 actionable tasks: 1 executed, 6 up-to-date


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

- Aurora ReviewBot


On Sept. 26, 2017, 8:50 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62590/
> ---
> 
> (Updated Sept. 26, 2017, 8:50 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Update to Thrift 0.10.0 (https://github.com/apache/thrift/blob/master/CHANGES)
> 
> Included changes:
> 
> * Switch to cmake rather than building with make. The cmake build is more 
> lenient with its build requirments. We would have needed to upgrade the 
> system bison on MacOS otherwise.
> * The Java `hashcode` option has been removed as it is now the default.
> 
> 
> ToDo / Questions:
> 
> * Rebuild the Aurora image to include Thrift 0.10. Until this is done, one 
> needs to manually run `sudo apt-get install flex bison cmake build-essential` 
> within the vagrant box to ensure Thrift can be bootstrapped.
> * The Gradle workaround looks strange. If it is really required we should 
> extract it into a separate patch.
> * The Pants build is complaining about the usage of `new_style` option. See 
> if we can get to work around it.
> * Do we want to maintain some patches to ensure we are just building the 
> required Thrift subset? Is this worth the effort?
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 0c04b98e791c5de1985724304f384e8dbade8c19 
>   build-support/packer/build.sh 7b7914c11e376a5f7ba7a5ee69fa0962e9bca379 
>   build-support/thrift/AURORA-1727.compiler.cpp.Makefile.in.patch 
> 77c966caa3d1f644241bcc2b1968bc9306c56689 
>   
> build-support/thrift/AURORA-1727.compiler.cpp.src.generate.t_java_generator.cc.patch
>  42300b43a8f72e45c96b975e5d3a6a7bd0283529 
>   build-support/thrift/AURORA-1727.lib.py.setup.py.patch 
> 11c7b13341e156f3686511cb40ab13c1256203a6 
>   build-support/thrift/Makefile f440b610afe321af663e393a29eebda7af7bd7a8 
>   build.gradle 851bab52fd389bc9577a6938f646bb29bae06139 
>   buildSrc/src/main/groovy/org/apache/aurora/build/ThriftPlugin.groovy 
> fc2bc9082dae2c63aa578c05dc89feb346260a67 
>   gradle.properties PRE-CREATION 
>   pants.ini f25e1d1a50f457325dbbabf327c31d760cb90339 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/Util.java 
> c28fb65010af5e3db925487929d4e0e12b4101a4 
> 
> 
> Diff: https://reviews.apache.org/r/62590/diff/1/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>