aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From John Sirois <jsir...@apache.org>
Subject Re: Review Request 53836: Get pants using the same thrift binary as gradle.
Date Wed, 07 Dec 2016 00:09:36 GMT


> On Nov. 17, 2016, 2:48 p.m., Zameer Manji wrote:
> > api/src/main/thrift/org/apache/aurora/gen/BUILD, line 34
> > <https://reviews.apache.org/r/53836/diff/1/?file=1565790#file1565790line34>
> >
> >     Can't we have our own 3rdparty thrift target that includes the thrift lib and
this prepare binary? Then we can put the dep in one place.
> 
> John Sirois wrote:
>     I can, but this requires a custom pants plugin (housed as a loose python sourcefile
in the repo) to expose, say, an `aurora_py_thrift_lib` target.  Your comments here expressed
reservations about that: https://reviews.apache.org/r/40219/
>     If you re-ack you're in-fact OK with a custom plugin for this, I'll whip up diff
2.
> 
> Zameer Manji wrote:
>     I think in this case, a custom pants plugin will be better, assuming the API in this
version is stable until pants 2.x
> 
> John Sirois wrote:
>     Yes, the API gaurantees are strong with pants since 1.0.0 and the plugin APIs needed
for this will be stable until 2.x.

Re-reading what you suggest which of course requires no plugin, simply adding the dep to target
at address `3rdparty/python:thrift` is straightforward.  Will do.


- John


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


On Nov. 16, 2016, 9:46 p.m., John Sirois wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53836/
> -----------------------------------------------------------
> 
> (Updated Nov. 16, 2016, 9:46 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This required an upgrade to the latest pants dev release to correct
> an issue with the setup.py binary packager we use to generate sdists.
> 
> This is for sanity sake, and, once the TODO in
> `build-support/thrift/prepare_binary.sh` is addressed, it will also
> allow pants-patch-free addition of new platforms (thinking ARM).
> 
>  api/src/main/thrift/org/apache/aurora/gen/BUILD                        |  3 +++
>  api/src/main/thrift/org/apache/thermos/BUILD                           |  1 +
>  build-support/thrift/.gitignore                                        |  1 +
>  build-support/thrift/AURORA-1727.compiler.cpp.Makefile.in.patch        | 42 +++++++++++++------------------
>  build-support/thrift/AURORA-1727.lib.py.setup.py.patch                 | 45 +++++++++++++++++++++++++++++++++
>  {api/src/main/thrift/org/apache/thermos => build-support/thrift}/BUILD | 18 ++++---------
>  build-support/thrift/Makefile                                          |  2 +-
>  build-support/thrift/prepare_binary.sh                                 | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  build-support/thrift/thriftw                                           | 12 +++++++--
>  pants.ini                                                              |  7 +++++-
>  10 files changed, 158 insertions(+), 41 deletions(-)
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/BUILD f4004a3ca36078c6d24991db4beb68903a05652c

>   api/src/main/thrift/org/apache/thermos/BUILD cb655a2fd35d22f7d7e80c4311742bad763d8614

>   build-support/thrift/.gitignore 9a3adb69210ba3dbf3c1f408895561da37e8f4c3 
>   build-support/thrift/AURORA-1727.compiler.cpp.Makefile.in.patch b69e3fef137cd73c6f2b73201463a0705ef8082a

>   build-support/thrift/AURORA-1727.lib.py.setup.py.patch PRE-CREATION 
>   build-support/thrift/BUILD PRE-CREATION 
>   build-support/thrift/Makefile 48b174ad622288d2738a5fa37bbb72385fcc3a27 
>   build-support/thrift/prepare_binary.sh PRE-CREATION 
>   build-support/thrift/thriftw 50d6dfdeb16ca8bf14aaff7aa826e3d69c5e13f0 
>   pants.ini cecdb277f327f77b2652f76a30fc8d4ffd9ff1db 
> 
> Diff: https://reviews.apache.org/r/53836/diff/
> 
> 
> Testing
> -------
> 
> Locally green:
> ```
> rm -rf ~/.cache/pants .cache/
> git clean -fdx build-support/thrift
> ./pants clean-all
> ./build-support/jenkins/build.sh
> 
> vagrant ssh --command "rm -rf ~/.cache/pants && cd aurora && rm -rf .cache/
&& ./pants clean-all"
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> ./build-support/python/make-pycharm-virtualenv
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>


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