You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Bill Farner <wf...@apache.org> on 2015/07/22 20:50:02 UTC

Review Request 36700: Remove binary build targets that are not currently for general consumption.

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

Review request for Aurora, Kevin Sweeney and Brian Wickman.


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


Repository: aurora


Description
-------

The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging).  This removes targets that Aurora users should not be consuming.


Diffs
-----

  build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 
  build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 
  src/main/python/apache/thermos/BUILD 8221aa0bd4efe5f519550cba716d6a564ba9ae44 
  src/main/python/apache/thermos/cli/bin/BUILD f33c7f838a6a0932abc737d0ecf7ca3a59580e2e 
  src/main/python/apache/thermos/observer/BUILD 41db2cc2e11234c434f58f55abec0b9f308096be 
  src/main/python/apache/thermos/observer/bin/BUILD 0abe2ccfe9c5ccb509ad731125385900114ba352 

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


Testing
-------


Thanks,

Bill Farner


Re: Review Request 36700: Remove binary build targets that are not currently for general consumption.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36700/#review92633
-----------------------------------------------------------


Master (b64f7b6) 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 July 22, 2015, 6:50 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36700/
> -----------------------------------------------------------
> 
> (Updated July 22, 2015, 6:50 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
>     https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging).  This removes targets that Aurora users should not be consuming.
> 
> 
> Diffs
> -----
> 
>   build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 
>   build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 
>   src/main/python/apache/thermos/BUILD 8221aa0bd4efe5f519550cba716d6a564ba9ae44 
>   src/main/python/apache/thermos/cli/bin/BUILD f33c7f838a6a0932abc737d0ecf7ca3a59580e2e 
>   src/main/python/apache/thermos/observer/BUILD 41db2cc2e11234c434f58f55abec0b9f308096be 
>   src/main/python/apache/thermos/observer/bin/BUILD 0abe2ccfe9c5ccb509ad731125385900114ba352 
> 
> Diff: https://reviews.apache.org/r/36700/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


Re: Review Request 36700: Remove binary build targets that are not currently for general consumption.

Posted by Kevin Sweeney <ke...@apache.org>.

> On July 24, 2015, 10:57 a.m., Brian Wickman wrote:
> > src/main/python/apache/thermos/observer/BUILD, line 74
> > <https://reviews.apache.org/r/36700/diff/2/?file=1020365#file1020365line74>
> >
> >     This puts an aurora dependency on thermos and I imagine causes a cycle in the build graph (pants may just silently ignore it.)
> >     
> >     Let's just remove the thermos_observer entry point from this package entirely by deleting the .with_binaries clause.
> 
> Bill Farner wrote:
>     This target (`src/main/python/apache/thermos/observer`) is a top-level target, only used by `build-support/release/make-python-sdists`, so i think we're safe from a cycle.  I'm not strictly against removing the `with_binaries`, but i assume it is there for a reason and something would break?  Or do we just not use the sdists [this way]?
> 
> Brian Wickman wrote:
>     Afaik nobody uses these in practice.  They're only useful for when you install the sdists via pip or pex directly.  Instead we always build pex binaries using pants which delegates to the python_binary target.

I answered someone's question in IRC with a gist ~2yrs ago https://gist.github.com/kevints/8361414 so it does seem likely these are used in practice. Okay with removing them but a NEWS entry would be good form.


- Kevin


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


On July 23, 2015, 5 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36700/
> -----------------------------------------------------------
> 
> (Updated July 23, 2015, 5 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
>     https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging).  This removes targets that Aurora users should not be consuming.
> 
> 
> Diffs
> -----
> 
>   build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 
>   build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 
>   src/main/python/apache/thermos/BUILD 8221aa0bd4efe5f519550cba716d6a564ba9ae44 
>   src/main/python/apache/thermos/cli/bin/BUILD f33c7f838a6a0932abc737d0ecf7ca3a59580e2e 
>   src/main/python/apache/thermos/cli/bin/thermos.py fcef38d33e7ca2005f35c3bdb90ffee6aeade3af 
>   src/main/python/apache/thermos/observer/BUILD 41db2cc2e11234c434f58f55abec0b9f308096be 
>   src/main/python/apache/thermos/observer/bin/BUILD 0abe2ccfe9c5ccb509ad731125385900114ba352 
>   src/main/python/apache/thermos/observer/bin/thermos_observer.py 39d3994a6163746e853cd21fc4c3585edc2b54cb 
> 
> Diff: https://reviews.apache.org/r/36700/diff/
> 
> 
> Testing
> -------
> 
> ./build-support/jenkins/build.sh
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> (as far as it currently gets on master, anyhow, due to AURORA-1409)
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


Re: Review Request 36700: Remove binary build targets that are not currently for general consumption.

Posted by Kevin Sweeney <ke...@apache.org>.

> On July 24, 2015, 10:57 a.m., Brian Wickman wrote:
> > src/main/python/apache/thermos/observer/BUILD, line 74
> > <https://reviews.apache.org/r/36700/diff/2/?file=1020365#file1020365line74>
> >
> >     This puts an aurora dependency on thermos and I imagine causes a cycle in the build graph (pants may just silently ignore it.)
> >     
> >     Let's just remove the thermos_observer entry point from this package entirely by deleting the .with_binaries clause.
> 
> Bill Farner wrote:
>     This target (`src/main/python/apache/thermos/observer`) is a top-level target, only used by `build-support/release/make-python-sdists`, so i think we're safe from a cycle.  I'm not strictly against removing the `with_binaries`, but i assume it is there for a reason and something would break?  Or do we just not use the sdists [this way]?
> 
> Brian Wickman wrote:
>     Afaik nobody uses these in practice.  They're only useful for when you install the sdists via pip or pex directly.  Instead we always build pex binaries using pants which delegates to the python_binary target.
> 
> Kevin Sweeney wrote:
>     I answered someone's question in IRC with a gist ~2yrs ago https://gist.github.com/kevints/8361414 so it does seem likely these are used in practice. Okay with removing them but a NEWS entry would be good form.
> 
> Bill Farner wrote:
>     Should i just kill the sdists entirely?
> 
> Brian Wickman wrote:
>     I'd prefer to go in the opposite direction and kill pants.
> 
> Bill Farner wrote:
>     I'd like to find a path to land a fix as packaging is currently broken/blocked.  I believe the current diff is the closest thing to compromise between the various opinions around this change.  Please let me know what you think.

-1 as this fix breaks the observer entry point entirely and produces an sdist that is invalid

```
(observer.venv)~aurora git aurora/. 36700    
% pip install -f dist apache.thermos.observer --pre                   
You are using pip version 6.1.1, however version 7.1.0 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.
Collecting apache.thermos.observer
  Could not find a version that satisfies the requirement apache.thermos.observer (from versions: 0.10.0-SNAPSHOT)
  No matching distribution found for apache.thermos.observer
```

so pants writes this sdist with a dependency on itself

since it's already broken I'd suggest removing the entry point entirely (so we're not advertising a feature we don't actually have).


- Kevin


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


On July 23, 2015, 5 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36700/
> -----------------------------------------------------------
> 
> (Updated July 23, 2015, 5 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
>     https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging).  This removes targets that Aurora users should not be consuming.
> 
> 
> Diffs
> -----
> 
>   build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 
>   build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 
>   src/main/python/apache/thermos/BUILD 8221aa0bd4efe5f519550cba716d6a564ba9ae44 
>   src/main/python/apache/thermos/cli/bin/BUILD f33c7f838a6a0932abc737d0ecf7ca3a59580e2e 
>   src/main/python/apache/thermos/cli/bin/thermos.py fcef38d33e7ca2005f35c3bdb90ffee6aeade3af 
>   src/main/python/apache/thermos/observer/BUILD 41db2cc2e11234c434f58f55abec0b9f308096be 
>   src/main/python/apache/thermos/observer/bin/BUILD 0abe2ccfe9c5ccb509ad731125385900114ba352 
>   src/main/python/apache/thermos/observer/bin/thermos_observer.py 39d3994a6163746e853cd21fc4c3585edc2b54cb 
> 
> Diff: https://reviews.apache.org/r/36700/diff/
> 
> 
> Testing
> -------
> 
> ./build-support/jenkins/build.sh
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> (as far as it currently gets on master, anyhow, due to AURORA-1409)
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


Re: Review Request 36700: Remove binary build targets that are not currently for general consumption.

Posted by Kevin Sweeney <ks...@twitter.com.INVALID>.
Me too, but I'm okay with killing the sdist targets as an intermediate step
and reintroducing them with a non-pants build

On Friday, July 24, 2015, Brian Wickman <wi...@apache.org> wrote:

>
>
> > On July 24, 2015, 5:57 p.m., Brian Wickman wrote:
> > > src/main/python/apache/thermos/observer/BUILD, line 74
> > > <
> https://reviews.apache.org/r/36700/diff/2/?file=1020365#file1020365line74>
> > >
> > >     This puts an aurora dependency on thermos and I imagine causes a
> cycle in the build graph (pants may just silently ignore it.)
> > >
> > >     Let's just remove the thermos_observer entry point from this
> package entirely by deleting the .with_binaries clause.
> >
> > Bill Farner wrote:
> >     This target (`src/main/python/apache/thermos/observer`) is a
> top-level target, only used by `build-support/release/make-python-sdists`,
> so i think we're safe from a cycle.  I'm not strictly against removing the
> `with_binaries`, but i assume it is there for a reason and something would
> break?  Or do we just not use the sdists [this way]?
> >
> > Brian Wickman wrote:
> >     Afaik nobody uses these in practice.  They're only useful for when
> you install the sdists via pip or pex directly.  Instead we always build
> pex binaries using pants which delegates to the python_binary target.
> >
> > Kevin Sweeney wrote:
> >     I answered someone's question in IRC with a gist ~2yrs ago
> https://gist.github.com/kevints/8361414 so it does seem likely these are
> used in practice. Okay with removing them but a NEWS entry would be good
> form.
> >
> > Bill Farner wrote:
> >     Should i just kill the sdists entirely?
>
> I'd prefer to go in the opposite direction and kill pants.
>
>
> - Brian
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36700/#review92944
> -----------------------------------------------------------
>
>
> On July 24, 2015, midnight, Bill Farner wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/36700/
> > -----------------------------------------------------------
> >
> > (Updated July 24, 2015, midnight)
> >
> >
> > Review request for Aurora, Kevin Sweeney and Brian Wickman.
> >
> >
> > Bugs: AURORA-1381
> >     https://issues.apache.org/jira/browse/AURORA-1381
> >
> >
> > Repository: aurora
> >
> >
> > Description
> > -------
> >
> > The meaning of the targets under
> `src/main/python/apache/thermos/cli/bin/BUILD` and
> `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which
> bit several people (including our own packaging).  This removes targets
> that Aurora users should not be consuming.
> >
> >
> > Diffs
> > -----
> >
> >   build-support/packaging/debian/rules
> 23828c02b73f007393d2ed4ce69c010cebf07e57
> >   build-support/packaging/rpm/aurora.spec
> 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771
> >   src/main/python/apache/thermos/BUILD
> 8221aa0bd4efe5f519550cba716d6a564ba9ae44
> >   src/main/python/apache/thermos/cli/bin/BUILD
> f33c7f838a6a0932abc737d0ecf7ca3a59580e2e
> >   src/main/python/apache/thermos/cli/bin/thermos.py
> fcef38d33e7ca2005f35c3bdb90ffee6aeade3af
> >   src/main/python/apache/thermos/observer/BUILD
> 41db2cc2e11234c434f58f55abec0b9f308096be
> >   src/main/python/apache/thermos/observer/bin/BUILD
> 0abe2ccfe9c5ccb509ad731125385900114ba352
> >   src/main/python/apache/thermos/observer/bin/thermos_observer.py
> 39d3994a6163746e853cd21fc4c3585edc2b54cb
> >
> > Diff: https://reviews.apache.org/r/36700/diff/
> >
> >
> > Testing
> > -------
> >
> > ./build-support/jenkins/build.sh
> >
> > ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> > (as far as it currently gets on master, anyhow, due to AURORA-1409)
> >
> >
> > Thanks,
> >
> > Bill Farner
> >
> >
>
>

-- 
Sent from Gmail Mobile

Re: Review Request 36700: Remove binary build targets that are not currently for general consumption.

Posted by Brian Wickman <wi...@apache.org>.

> On July 24, 2015, 5:57 p.m., Brian Wickman wrote:
> > src/main/python/apache/thermos/observer/BUILD, line 74
> > <https://reviews.apache.org/r/36700/diff/2/?file=1020365#file1020365line74>
> >
> >     This puts an aurora dependency on thermos and I imagine causes a cycle in the build graph (pants may just silently ignore it.)
> >     
> >     Let's just remove the thermos_observer entry point from this package entirely by deleting the .with_binaries clause.
> 
> Bill Farner wrote:
>     This target (`src/main/python/apache/thermos/observer`) is a top-level target, only used by `build-support/release/make-python-sdists`, so i think we're safe from a cycle.  I'm not strictly against removing the `with_binaries`, but i assume it is there for a reason and something would break?  Or do we just not use the sdists [this way]?
> 
> Brian Wickman wrote:
>     Afaik nobody uses these in practice.  They're only useful for when you install the sdists via pip or pex directly.  Instead we always build pex binaries using pants which delegates to the python_binary target.
> 
> Kevin Sweeney wrote:
>     I answered someone's question in IRC with a gist ~2yrs ago https://gist.github.com/kevints/8361414 so it does seem likely these are used in practice. Okay with removing them but a NEWS entry would be good form.
> 
> Bill Farner wrote:
>     Should i just kill the sdists entirely?

I'd prefer to go in the opposite direction and kill pants.


- Brian


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


On July 24, 2015, midnight, Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36700/
> -----------------------------------------------------------
> 
> (Updated July 24, 2015, midnight)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
>     https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging).  This removes targets that Aurora users should not be consuming.
> 
> 
> Diffs
> -----
> 
>   build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 
>   build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 
>   src/main/python/apache/thermos/BUILD 8221aa0bd4efe5f519550cba716d6a564ba9ae44 
>   src/main/python/apache/thermos/cli/bin/BUILD f33c7f838a6a0932abc737d0ecf7ca3a59580e2e 
>   src/main/python/apache/thermos/cli/bin/thermos.py fcef38d33e7ca2005f35c3bdb90ffee6aeade3af 
>   src/main/python/apache/thermos/observer/BUILD 41db2cc2e11234c434f58f55abec0b9f308096be 
>   src/main/python/apache/thermos/observer/bin/BUILD 0abe2ccfe9c5ccb509ad731125385900114ba352 
>   src/main/python/apache/thermos/observer/bin/thermos_observer.py 39d3994a6163746e853cd21fc4c3585edc2b54cb 
> 
> Diff: https://reviews.apache.org/r/36700/diff/
> 
> 
> Testing
> -------
> 
> ./build-support/jenkins/build.sh
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> (as far as it currently gets on master, anyhow, due to AURORA-1409)
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


Re: Review Request 36700: Remove binary build targets that are not currently for general consumption.

Posted by Bill Farner <wf...@apache.org>.

> On July 24, 2015, 5:57 p.m., Brian Wickman wrote:
> > src/main/python/apache/thermos/observer/BUILD, line 74
> > <https://reviews.apache.org/r/36700/diff/2/?file=1020365#file1020365line74>
> >
> >     This puts an aurora dependency on thermos and I imagine causes a cycle in the build graph (pants may just silently ignore it.)
> >     
> >     Let's just remove the thermos_observer entry point from this package entirely by deleting the .with_binaries clause.
> 
> Bill Farner wrote:
>     This target (`src/main/python/apache/thermos/observer`) is a top-level target, only used by `build-support/release/make-python-sdists`, so i think we're safe from a cycle.  I'm not strictly against removing the `with_binaries`, but i assume it is there for a reason and something would break?  Or do we just not use the sdists [this way]?
> 
> Brian Wickman wrote:
>     Afaik nobody uses these in practice.  They're only useful for when you install the sdists via pip or pex directly.  Instead we always build pex binaries using pants which delegates to the python_binary target.
> 
> Kevin Sweeney wrote:
>     I answered someone's question in IRC with a gist ~2yrs ago https://gist.github.com/kevints/8361414 so it does seem likely these are used in practice. Okay with removing them but a NEWS entry would be good form.
> 
> Bill Farner wrote:
>     Should i just kill the sdists entirely?
> 
> Brian Wickman wrote:
>     I'd prefer to go in the opposite direction and kill pants.

I'd like to find a path to land a fix as packaging is currently broken/blocked.  I believe the current diff is the closest thing to compromise between the various opinions around this change.  Please let me know what you think.


- Bill


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


On July 24, 2015, midnight, Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36700/
> -----------------------------------------------------------
> 
> (Updated July 24, 2015, midnight)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
>     https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging).  This removes targets that Aurora users should not be consuming.
> 
> 
> Diffs
> -----
> 
>   build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 
>   build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 
>   src/main/python/apache/thermos/BUILD 8221aa0bd4efe5f519550cba716d6a564ba9ae44 
>   src/main/python/apache/thermos/cli/bin/BUILD f33c7f838a6a0932abc737d0ecf7ca3a59580e2e 
>   src/main/python/apache/thermos/cli/bin/thermos.py fcef38d33e7ca2005f35c3bdb90ffee6aeade3af 
>   src/main/python/apache/thermos/observer/BUILD 41db2cc2e11234c434f58f55abec0b9f308096be 
>   src/main/python/apache/thermos/observer/bin/BUILD 0abe2ccfe9c5ccb509ad731125385900114ba352 
>   src/main/python/apache/thermos/observer/bin/thermos_observer.py 39d3994a6163746e853cd21fc4c3585edc2b54cb 
> 
> Diff: https://reviews.apache.org/r/36700/diff/
> 
> 
> Testing
> -------
> 
> ./build-support/jenkins/build.sh
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> (as far as it currently gets on master, anyhow, due to AURORA-1409)
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


Re: Review Request 36700: Remove binary build targets that are not currently for general consumption.

Posted by Bill Farner <wf...@apache.org>.

> On July 24, 2015, 5:57 p.m., Brian Wickman wrote:
> > src/main/python/apache/thermos/observer/BUILD, line 74
> > <https://reviews.apache.org/r/36700/diff/2/?file=1020365#file1020365line74>
> >
> >     This puts an aurora dependency on thermos and I imagine causes a cycle in the build graph (pants may just silently ignore it.)
> >     
> >     Let's just remove the thermos_observer entry point from this package entirely by deleting the .with_binaries clause.

This target (`src/main/python/apache/thermos/observer`) is a top-level target, only used by `build-support/release/make-python-sdists`, so i think we're safe from a cycle.  I'm not strictly against removing the `with_binaries`, but i assume it is there for a reason and something would break?  Or do we just not use the sdists [this way]?


- Bill


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


On July 24, 2015, midnight, Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36700/
> -----------------------------------------------------------
> 
> (Updated July 24, 2015, midnight)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
>     https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging).  This removes targets that Aurora users should not be consuming.
> 
> 
> Diffs
> -----
> 
>   build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 
>   build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 
>   src/main/python/apache/thermos/BUILD 8221aa0bd4efe5f519550cba716d6a564ba9ae44 
>   src/main/python/apache/thermos/cli/bin/BUILD f33c7f838a6a0932abc737d0ecf7ca3a59580e2e 
>   src/main/python/apache/thermos/cli/bin/thermos.py fcef38d33e7ca2005f35c3bdb90ffee6aeade3af 
>   src/main/python/apache/thermos/observer/BUILD 41db2cc2e11234c434f58f55abec0b9f308096be 
>   src/main/python/apache/thermos/observer/bin/BUILD 0abe2ccfe9c5ccb509ad731125385900114ba352 
>   src/main/python/apache/thermos/observer/bin/thermos_observer.py 39d3994a6163746e853cd21fc4c3585edc2b54cb 
> 
> Diff: https://reviews.apache.org/r/36700/diff/
> 
> 
> Testing
> -------
> 
> ./build-support/jenkins/build.sh
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> (as far as it currently gets on master, anyhow, due to AURORA-1409)
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


Re: Review Request 36700: Remove binary build targets that are not currently for general consumption.

Posted by Bill Farner <wf...@apache.org>.

> On July 24, 2015, 5:57 p.m., Brian Wickman wrote:
> > src/main/python/apache/thermos/observer/BUILD, line 74
> > <https://reviews.apache.org/r/36700/diff/2/?file=1020365#file1020365line74>
> >
> >     This puts an aurora dependency on thermos and I imagine causes a cycle in the build graph (pants may just silently ignore it.)
> >     
> >     Let's just remove the thermos_observer entry point from this package entirely by deleting the .with_binaries clause.
> 
> Bill Farner wrote:
>     This target (`src/main/python/apache/thermos/observer`) is a top-level target, only used by `build-support/release/make-python-sdists`, so i think we're safe from a cycle.  I'm not strictly against removing the `with_binaries`, but i assume it is there for a reason and something would break?  Or do we just not use the sdists [this way]?
> 
> Brian Wickman wrote:
>     Afaik nobody uses these in practice.  They're only useful for when you install the sdists via pip or pex directly.  Instead we always build pex binaries using pants which delegates to the python_binary target.
> 
> Kevin Sweeney wrote:
>     I answered someone's question in IRC with a gist ~2yrs ago https://gist.github.com/kevints/8361414 so it does seem likely these are used in practice. Okay with removing them but a NEWS entry would be good form.

Should i just kill the sdists entirely?


- Bill


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


On July 24, 2015, midnight, Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36700/
> -----------------------------------------------------------
> 
> (Updated July 24, 2015, midnight)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
>     https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging).  This removes targets that Aurora users should not be consuming.
> 
> 
> Diffs
> -----
> 
>   build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 
>   build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 
>   src/main/python/apache/thermos/BUILD 8221aa0bd4efe5f519550cba716d6a564ba9ae44 
>   src/main/python/apache/thermos/cli/bin/BUILD f33c7f838a6a0932abc737d0ecf7ca3a59580e2e 
>   src/main/python/apache/thermos/cli/bin/thermos.py fcef38d33e7ca2005f35c3bdb90ffee6aeade3af 
>   src/main/python/apache/thermos/observer/BUILD 41db2cc2e11234c434f58f55abec0b9f308096be 
>   src/main/python/apache/thermos/observer/bin/BUILD 0abe2ccfe9c5ccb509ad731125385900114ba352 
>   src/main/python/apache/thermos/observer/bin/thermos_observer.py 39d3994a6163746e853cd21fc4c3585edc2b54cb 
> 
> Diff: https://reviews.apache.org/r/36700/diff/
> 
> 
> Testing
> -------
> 
> ./build-support/jenkins/build.sh
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> (as far as it currently gets on master, anyhow, due to AURORA-1409)
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


Re: Review Request 36700: Remove binary build targets that are not currently for general consumption.

Posted by Brian Wickman <wi...@apache.org>.

> On July 24, 2015, 5:57 p.m., Brian Wickman wrote:
> > src/main/python/apache/thermos/observer/BUILD, line 74
> > <https://reviews.apache.org/r/36700/diff/2/?file=1020365#file1020365line74>
> >
> >     This puts an aurora dependency on thermos and I imagine causes a cycle in the build graph (pants may just silently ignore it.)
> >     
> >     Let's just remove the thermos_observer entry point from this package entirely by deleting the .with_binaries clause.
> 
> Bill Farner wrote:
>     This target (`src/main/python/apache/thermos/observer`) is a top-level target, only used by `build-support/release/make-python-sdists`, so i think we're safe from a cycle.  I'm not strictly against removing the `with_binaries`, but i assume it is there for a reason and something would break?  Or do we just not use the sdists [this way]?

Afaik nobody uses these in practice.  They're only useful for when you install the sdists via pip or pex directly.  Instead we always build pex binaries using pants which delegates to the python_binary target.


- Brian


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


On July 24, 2015, midnight, Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36700/
> -----------------------------------------------------------
> 
> (Updated July 24, 2015, midnight)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
>     https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging).  This removes targets that Aurora users should not be consuming.
> 
> 
> Diffs
> -----
> 
>   build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 
>   build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 
>   src/main/python/apache/thermos/BUILD 8221aa0bd4efe5f519550cba716d6a564ba9ae44 
>   src/main/python/apache/thermos/cli/bin/BUILD f33c7f838a6a0932abc737d0ecf7ca3a59580e2e 
>   src/main/python/apache/thermos/cli/bin/thermos.py fcef38d33e7ca2005f35c3bdb90ffee6aeade3af 
>   src/main/python/apache/thermos/observer/BUILD 41db2cc2e11234c434f58f55abec0b9f308096be 
>   src/main/python/apache/thermos/observer/bin/BUILD 0abe2ccfe9c5ccb509ad731125385900114ba352 
>   src/main/python/apache/thermos/observer/bin/thermos_observer.py 39d3994a6163746e853cd21fc4c3585edc2b54cb 
> 
> Diff: https://reviews.apache.org/r/36700/diff/
> 
> 
> Testing
> -------
> 
> ./build-support/jenkins/build.sh
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> (as far as it currently gets on master, anyhow, due to AURORA-1409)
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


Re: Review Request 36700: Remove binary build targets that are not currently for general consumption.

Posted by Brian Wickman <wi...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36700/#review92944
-----------------------------------------------------------



src/main/python/apache/thermos/observer/BUILD (line 74)
<https://reviews.apache.org/r/36700/#comment147203>

    This puts an aurora dependency on thermos and I imagine causes a cycle in the build graph (pants may just silently ignore it.)
    
    Let's just remove the thermos_observer entry point from this package entirely by deleting the .with_binaries clause.


- Brian Wickman


On July 24, 2015, midnight, Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36700/
> -----------------------------------------------------------
> 
> (Updated July 24, 2015, midnight)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
>     https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging).  This removes targets that Aurora users should not be consuming.
> 
> 
> Diffs
> -----
> 
>   build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 
>   build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 
>   src/main/python/apache/thermos/BUILD 8221aa0bd4efe5f519550cba716d6a564ba9ae44 
>   src/main/python/apache/thermos/cli/bin/BUILD f33c7f838a6a0932abc737d0ecf7ca3a59580e2e 
>   src/main/python/apache/thermos/cli/bin/thermos.py fcef38d33e7ca2005f35c3bdb90ffee6aeade3af 
>   src/main/python/apache/thermos/observer/BUILD 41db2cc2e11234c434f58f55abec0b9f308096be 
>   src/main/python/apache/thermos/observer/bin/BUILD 0abe2ccfe9c5ccb509ad731125385900114ba352 
>   src/main/python/apache/thermos/observer/bin/thermos_observer.py 39d3994a6163746e853cd21fc4c3585edc2b54cb 
> 
> Diff: https://reviews.apache.org/r/36700/diff/
> 
> 
> Testing
> -------
> 
> ./build-support/jenkins/build.sh
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> (as far as it currently gets on master, anyhow, due to AURORA-1409)
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


Re: Review Request 36700: Remove references to binary build targets that are not currently for general consumption.

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36700/#review93590
-----------------------------------------------------------


FYI - i have peeled off https://reviews.apache.org/r/36945/ to avoid further engangling the packaging issue with the unused/confusing targets issue.

- Bill Farner


On July 28, 2015, 12:58 a.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36700/
> -----------------------------------------------------------
> 
> (Updated July 28, 2015, 12:58 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
>     https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging).  This removes targets that Aurora users should not be consuming.
> 
> 
> Diffs
> -----
> 
>   README.md f3b23247cf3f7e550c1714b4fb8227a2baab3b41 
>   build-support/jenkins/build.sh 5606bb157cb117a588f363382d7c8841ae957138 
>   build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 
>   build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 
>   build-support/release/make-python-sdists 9608f68e16243da01434ce2fc7d61bb7c7efd712 
> 
> Diff: https://reviews.apache.org/r/36700/diff/
> 
> 
> Testing
> -------
> 
> ./build-support/jenkins/build.sh
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> (as far as it currently gets on master, anyhow, due to AURORA-1409)
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


Re: Review Request 36700: Remove references to binary build targets that are not currently for general consumption.

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36700/#review93624
-----------------------------------------------------------


I'm abandoning this review.  The time spent trying to achieve consensus has exceeded the value.  I suggest users look to forthcoming binary packages to insulate from build refactors in the future.

- Bill Farner


On July 28, 2015, 12:58 a.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36700/
> -----------------------------------------------------------
> 
> (Updated July 28, 2015, 12:58 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
>     https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging).  This removes targets that Aurora users should not be consuming.
> 
> 
> Diffs
> -----
> 
>   README.md f3b23247cf3f7e550c1714b4fb8227a2baab3b41 
>   build-support/jenkins/build.sh 5606bb157cb117a588f363382d7c8841ae957138 
>   build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 
>   build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 
>   build-support/release/make-python-sdists 9608f68e16243da01434ce2fc7d61bb7c7efd712 
> 
> Diff: https://reviews.apache.org/r/36700/diff/
> 
> 
> Testing
> -------
> 
> ./build-support/jenkins/build.sh
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> (as far as it currently gets on master, anyhow, due to AURORA-1409)
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


Re: Review Request 36700: Remove references to binary build targets that are not currently for general consumption.

Posted by Bill Farner <wf...@apache.org>.

> On July 28, 2015, 1:01 a.m., Kevin Sweeney wrote:
> > README.md, line 127
> > <https://reviews.apache.org/r/36700/diff/3/?file=1023270#file1023270line127>
> >
> >     Is there any reason to remove the `./gradlew distTar` guidance?

I just concluded that we're better off not telling people how to build any binaries rather than just one of the several pieces they'll need.  What's your take?


- Bill


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


On July 28, 2015, 12:58 a.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36700/
> -----------------------------------------------------------
> 
> (Updated July 28, 2015, 12:58 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
>     https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging).  This removes targets that Aurora users should not be consuming.
> 
> 
> Diffs
> -----
> 
>   README.md f3b23247cf3f7e550c1714b4fb8227a2baab3b41 
>   build-support/jenkins/build.sh 5606bb157cb117a588f363382d7c8841ae957138 
>   build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 
>   build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 
>   build-support/release/make-python-sdists 9608f68e16243da01434ce2fc7d61bb7c7efd712 
> 
> Diff: https://reviews.apache.org/r/36700/diff/
> 
> 
> Testing
> -------
> 
> ./build-support/jenkins/build.sh
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> (as far as it currently gets on master, anyhow, due to AURORA-1409)
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


Re: Review Request 36700: Remove references to binary build targets that are not currently for general consumption.

Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36700/#review93211
-----------------------------------------------------------



README.md 
<https://reviews.apache.org/r/36700/#comment147530>

    Is there any reason to remove the `./gradlew distTar` guidance?


- Kevin Sweeney


On July 27, 2015, 5:58 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36700/
> -----------------------------------------------------------
> 
> (Updated July 27, 2015, 5:58 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
>     https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging).  This removes targets that Aurora users should not be consuming.
> 
> 
> Diffs
> -----
> 
>   README.md f3b23247cf3f7e550c1714b4fb8227a2baab3b41 
>   build-support/jenkins/build.sh 5606bb157cb117a588f363382d7c8841ae957138 
>   build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 
>   build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 
>   build-support/release/make-python-sdists 9608f68e16243da01434ce2fc7d61bb7c7efd712 
> 
> Diff: https://reviews.apache.org/r/36700/diff/
> 
> 
> Testing
> -------
> 
> ./build-support/jenkins/build.sh
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> (as far as it currently gets on master, anyhow, due to AURORA-1409)
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


Re: Review Request 36700: Remove references to binary build targets that are not currently for general consumption.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36700/#review93214
-----------------------------------------------------------

Ship it!


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

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

- Aurora ReviewBot


On July 28, 2015, 12:58 a.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36700/
> -----------------------------------------------------------
> 
> (Updated July 28, 2015, 12:58 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
>     https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging).  This removes targets that Aurora users should not be consuming.
> 
> 
> Diffs
> -----
> 
>   README.md f3b23247cf3f7e550c1714b4fb8227a2baab3b41 
>   build-support/jenkins/build.sh 5606bb157cb117a588f363382d7c8841ae957138 
>   build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 
>   build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 
>   build-support/release/make-python-sdists 9608f68e16243da01434ce2fc7d61bb7c7efd712 
> 
> Diff: https://reviews.apache.org/r/36700/diff/
> 
> 
> Testing
> -------
> 
> ./build-support/jenkins/build.sh
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> (as far as it currently gets on master, anyhow, due to AURORA-1409)
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


Re: Review Request 36700: Remove references to binary build targets that are not currently for general consumption.

Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36700/#review93209
-----------------------------------------------------------

Ship it!


Ship It!

- Kevin Sweeney


On July 27, 2015, 5:58 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36700/
> -----------------------------------------------------------
> 
> (Updated July 27, 2015, 5:58 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
>     https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging).  This removes targets that Aurora users should not be consuming.
> 
> 
> Diffs
> -----
> 
>   README.md f3b23247cf3f7e550c1714b4fb8227a2baab3b41 
>   build-support/jenkins/build.sh 5606bb157cb117a588f363382d7c8841ae957138 
>   build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 
>   build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 
>   build-support/release/make-python-sdists 9608f68e16243da01434ce2fc7d61bb7c7efd712 
> 
> Diff: https://reviews.apache.org/r/36700/diff/
> 
> 
> Testing
> -------
> 
> ./build-support/jenkins/build.sh
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> (as far as it currently gets on master, anyhow, due to AURORA-1409)
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


Re: Review Request 36700: Remove references to binary build targets that are not currently for general consumption.

Posted by Bill Farner <wf...@apache.org>.

> On July 28, 2015, 11:03 a.m., Stephan Erb wrote:
> > Please reconsider the removal of the `build-support/release/make-python-sdists` script.
> > 
> > We are using `apache.aurora.client` and its dependencies in production. We install the client sdist into a virtualenv and then use a thin wrapper around `apache.aurora.client.api.AuroraClientAPI` in order to control our cluster. Using the pex does neither seem to offer the same flexibility nor the same performance (subprocess call and thrift connections take rather long).

Thanks for the quick feedback, you saved me a trip to a mailing list.  I will re-assess what to do here.


- Bill


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


On July 28, 2015, 12:58 a.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36700/
> -----------------------------------------------------------
> 
> (Updated July 28, 2015, 12:58 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
>     https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging).  This removes targets that Aurora users should not be consuming.
> 
> 
> Diffs
> -----
> 
>   README.md f3b23247cf3f7e550c1714b4fb8227a2baab3b41 
>   build-support/jenkins/build.sh 5606bb157cb117a588f363382d7c8841ae957138 
>   build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 
>   build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 
>   build-support/release/make-python-sdists 9608f68e16243da01434ce2fc7d61bb7c7efd712 
> 
> Diff: https://reviews.apache.org/r/36700/diff/
> 
> 
> Testing
> -------
> 
> ./build-support/jenkins/build.sh
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> (as far as it currently gets on master, anyhow, due to AURORA-1409)
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


Re: Review Request 36700: Remove references to binary build targets that are not currently for general consumption.

Posted by Stephan Erb <st...@dev.static-void.de>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36700/#review93270
-----------------------------------------------------------


Please reconsider the removal of the `build-support/release/make-python-sdists` script.

We are using `apache.aurora.client` and its dependencies in production. We install the client sdist into a virtualenv and then use a thin wrapper around `apache.aurora.client.api.AuroraClientAPI` in order to control our cluster. Using the pex does neither seem to offer the same flexibility nor the same performance (subprocess call and thrift connections take rather long).

- Stephan Erb


On July 28, 2015, 2:58 a.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36700/
> -----------------------------------------------------------
> 
> (Updated July 28, 2015, 2:58 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
>     https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging).  This removes targets that Aurora users should not be consuming.
> 
> 
> Diffs
> -----
> 
>   README.md f3b23247cf3f7e550c1714b4fb8227a2baab3b41 
>   build-support/jenkins/build.sh 5606bb157cb117a588f363382d7c8841ae957138 
>   build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 
>   build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 
>   build-support/release/make-python-sdists 9608f68e16243da01434ce2fc7d61bb7c7efd712 
> 
> Diff: https://reviews.apache.org/r/36700/diff/
> 
> 
> Testing
> -------
> 
> ./build-support/jenkins/build.sh
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> (as far as it currently gets on master, anyhow, due to AURORA-1409)
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


Re: Review Request 36700: Remove references to binary build targets that are not currently for general consumption.

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36700/
-----------------------------------------------------------

(Updated July 28, 2015, 12:58 a.m.)


Review request for Aurora, Kevin Sweeney and Brian Wickman.


Changes
-------

Stepped back to removal of sdists, which are broken in several cases.  The current guidance is for users to consume python artifacts as pex files.  We may reintroduce pip-installable artifacts in the future with a change of build tools.


Summary (updated)
-----------------

Remove references to binary build targets that are not currently for general consumption.


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


Repository: aurora


Description
-------

The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging).  This removes targets that Aurora users should not be consuming.


Diffs (updated)
-----

  README.md f3b23247cf3f7e550c1714b4fb8227a2baab3b41 
  build-support/jenkins/build.sh 5606bb157cb117a588f363382d7c8841ae957138 
  build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 
  build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 
  build-support/release/make-python-sdists 9608f68e16243da01434ce2fc7d61bb7c7efd712 

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


Testing
-------

./build-support/jenkins/build.sh

./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
(as far as it currently gets on master, anyhow, due to AURORA-1409)


Thanks,

Bill Farner


Re: Review Request 36700: Remove binary build targets that are not currently for general consumption.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36700/#review92845
-----------------------------------------------------------


Master (24088ef) 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 July 24, 2015, midnight, Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36700/
> -----------------------------------------------------------
> 
> (Updated July 24, 2015, midnight)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
>     https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging).  This removes targets that Aurora users should not be consuming.
> 
> 
> Diffs
> -----
> 
>   build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 
>   build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 
>   src/main/python/apache/thermos/BUILD 8221aa0bd4efe5f519550cba716d6a564ba9ae44 
>   src/main/python/apache/thermos/cli/bin/BUILD f33c7f838a6a0932abc737d0ecf7ca3a59580e2e 
>   src/main/python/apache/thermos/cli/bin/thermos.py fcef38d33e7ca2005f35c3bdb90ffee6aeade3af 
>   src/main/python/apache/thermos/observer/BUILD 41db2cc2e11234c434f58f55abec0b9f308096be 
>   src/main/python/apache/thermos/observer/bin/BUILD 0abe2ccfe9c5ccb509ad731125385900114ba352 
>   src/main/python/apache/thermos/observer/bin/thermos_observer.py 39d3994a6163746e853cd21fc4c3585edc2b54cb 
> 
> Diff: https://reviews.apache.org/r/36700/diff/
> 
> 
> Testing
> -------
> 
> ./build-support/jenkins/build.sh
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> (as far as it currently gets on master, anyhow, due to AURORA-1409)
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


Re: Review Request 36700: Remove binary build targets that are not currently for general consumption.

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36700/
-----------------------------------------------------------

(Updated July 24, 2015, midnight)


Review request for Aurora, Kevin Sweeney and Brian Wickman.


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


Repository: aurora


Description
-------

The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging).  This removes targets that Aurora users should not be consuming.


Diffs (updated)
-----

  build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 
  build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 
  src/main/python/apache/thermos/BUILD 8221aa0bd4efe5f519550cba716d6a564ba9ae44 
  src/main/python/apache/thermos/cli/bin/BUILD f33c7f838a6a0932abc737d0ecf7ca3a59580e2e 
  src/main/python/apache/thermos/cli/bin/thermos.py fcef38d33e7ca2005f35c3bdb90ffee6aeade3af 
  src/main/python/apache/thermos/observer/BUILD 41db2cc2e11234c434f58f55abec0b9f308096be 
  src/main/python/apache/thermos/observer/bin/BUILD 0abe2ccfe9c5ccb509ad731125385900114ba352 
  src/main/python/apache/thermos/observer/bin/thermos_observer.py 39d3994a6163746e853cd21fc4c3585edc2b54cb 

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


Testing
-------

./build-support/jenkins/build.sh

./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
(as far as it currently gets on master, anyhow, due to AURORA-1409)


Thanks,

Bill Farner


Re: Review Request 36700: Remove binary build targets that are not currently for general consumption.

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36700/
-----------------------------------------------------------

(Updated July 24, 2015, midnight)


Review request for Aurora, Kevin Sweeney and Brian Wickman.


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


Repository: aurora


Description
-------

The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging).  This removes targets that Aurora users should not be consuming.


Diffs
-----

  build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 
  build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 
  src/main/python/apache/thermos/BUILD 8221aa0bd4efe5f519550cba716d6a564ba9ae44 
  src/main/python/apache/thermos/cli/bin/BUILD f33c7f838a6a0932abc737d0ecf7ca3a59580e2e 
  src/main/python/apache/thermos/observer/BUILD 41db2cc2e11234c434f58f55abec0b9f308096be 
  src/main/python/apache/thermos/observer/bin/BUILD 0abe2ccfe9c5ccb509ad731125385900114ba352 

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


Testing (updated)
-------

./build-support/jenkins/build.sh

./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
(as far as it currently gets on master, anyhow, due to AURORA-1409)


Thanks,

Bill Farner


Re: Review Request 36700: Remove binary build targets that are not currently for general consumption.

Posted by Bill Farner <wf...@apache.org>.

> On July 22, 2015, 7:56 p.m., Kevin Sweeney wrote:
> > build-support/packaging/debian/rules, line 37
> > <https://reviews.apache.org/r/36700/diff/1/?file=1018921#file1018921line37>
> >
> >     Can you delete the code these targets reference as well, as presumably there are now source files without BUILD file owners in our tree

Done.


- Bill


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


On July 22, 2015, 6:50 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36700/
> -----------------------------------------------------------
> 
> (Updated July 22, 2015, 6:50 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
>     https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging).  This removes targets that Aurora users should not be consuming.
> 
> 
> Diffs
> -----
> 
>   build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 
>   build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 
>   src/main/python/apache/thermos/BUILD 8221aa0bd4efe5f519550cba716d6a564ba9ae44 
>   src/main/python/apache/thermos/cli/bin/BUILD f33c7f838a6a0932abc737d0ecf7ca3a59580e2e 
>   src/main/python/apache/thermos/observer/BUILD 41db2cc2e11234c434f58f55abec0b9f308096be 
>   src/main/python/apache/thermos/observer/bin/BUILD 0abe2ccfe9c5ccb509ad731125385900114ba352 
> 
> Diff: https://reviews.apache.org/r/36700/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


Re: Review Request 36700: Remove binary build targets that are not currently for general consumption.

Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36700/#review92639
-----------------------------------------------------------



build-support/packaging/debian/rules (line 37)
<https://reviews.apache.org/r/36700/#comment146861>

    Can you delete the code these targets reference as well, as presumably there are now source files without BUILD file owners in our tree


- Kevin Sweeney


On July 22, 2015, 11:50 a.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36700/
> -----------------------------------------------------------
> 
> (Updated July 22, 2015, 11:50 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
>     https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The meaning of the targets under `src/main/python/apache/thermos/cli/bin/BUILD` and `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which bit several people (including our own packaging).  This removes targets that Aurora users should not be consuming.
> 
> 
> Diffs
> -----
> 
>   build-support/packaging/debian/rules 23828c02b73f007393d2ed4ce69c010cebf07e57 
>   build-support/packaging/rpm/aurora.spec 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 
>   src/main/python/apache/thermos/BUILD 8221aa0bd4efe5f519550cba716d6a564ba9ae44 
>   src/main/python/apache/thermos/cli/bin/BUILD f33c7f838a6a0932abc737d0ecf7ca3a59580e2e 
>   src/main/python/apache/thermos/observer/BUILD 41db2cc2e11234c434f58f55abec0b9f308096be 
>   src/main/python/apache/thermos/observer/bin/BUILD 0abe2ccfe9c5ccb509ad731125385900114ba352 
> 
> Diff: https://reviews.apache.org/r/36700/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bill Farner
> 
>