aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Farner" <wfar...@apache.org>
Subject Re: Review Request 30207: Simplify AuroraCommandContext
Date Mon, 26 Jan 2015 22:08:51 GMT


> On Jan. 23, 2015, 8:47 p.m., Maxim Khutornenko wrote:
> > I am not convinced there is enough value in this diff to risk possible regression.
Besides, the majority of what this diff touches will die out along with the client updater.
> 
> Zameer Manji wrote:
>     Is there an ETA for the destruction of the client updater?
> 
> Maxim Khutornenko wrote:
>     Any time we feel ready to drop "beta" from scheduler updater.
> 
> Zameer Manji wrote:
>     I'm willing to drop this diff, if you are willing to start the conversation on when
we can drop 'beta' from the scheduler updater.
> 
> Maxim Khutornenko wrote:
>     How is that related? :) There is nothing pressing us to graduate scheduler updater
at this point. There are still bugs to fix and parity features to implement (e.g. heartbeats)
before we are ready for prime time.
> 
> Bill Farner wrote:
>     I share Maxim's general uneasiness about changing behavior in the client-side updater
since it is complex and sunsetting.  However, i don't share the concern in this diff.  The
change appears to be very straightforward, especially in `update.py`.  Maxim - is there any
particular part you're worried about?
> 
> Maxim Khutornenko wrote:
>     I just don't see a reason to shuffle things around (no matter how trivial it looks)
for a feature that is going away. I view the value of refactoring as making a long term positive
impact on readabilty and reusability. This change does not clear the bar for me.

The important context is that this is not a refactor in service of the updater, but to unwind
context.py, which has very high fan-in within the client.


- Bill


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


On Jan. 23, 2015, 3:32 a.m., Zameer Manji wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30207/
> -----------------------------------------------------------
> 
> (Updated Jan. 23, 2015, 3:32 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The AuroraCommandContext class is used in multiple commands and contains common code
for all of them. However some portions are only used by one command. This patch takes some
of those portions and moves them to the command that requires that functionality.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/client/cli/context.py 51c7d24dca664e476e62f1864d095416dfab70e4

>   src/main/python/apache/aurora/client/cli/jobs.py 8f349c09637c16e2499e85f2dc96eb7ccffd0aaf

>   src/main/python/apache/aurora/client/cli/update.py PRE-CREATION 
>   src/test/python/apache/aurora/client/cli/test_supdate.py PRE-CREATION 
>   src/test/python/apache/aurora/client/cli/test_update.py 8b7d11202b35deb09a0000248cfe0a96458fb70c

> 
> Diff: https://reviews.apache.org/r/30207/diff/
> 
> 
> Testing
> -------
> 
> ./pants test.pytest --no-fast src/test/python/apache/aurora/client::
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>


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