> 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 > >
