Re: Review Request 53403: [WIP] [DO NOT MERGE] Reparent orphaned processes to thermos runner and send SIGTERM on task teardown

2016-11-02 Thread Joshua Cohen
is maybe more pythonic (assuming I got my syntax right anyway...) src/main/python/apache/thermos/core/helper.py (line 242) <https://reviews.apache.org/r/53403/#comment224297> I think this would probably make sense as an info log. - Joshua Cohen On Nov. 2, 2016, 10:16 p.m.,

Re: Review Request 52479: Resolve docker tags to concrete identifiers for DockerContainerizer

2016-11-02 Thread Joshua Cohen
s://reviews.apache.org/r/52479/ > --- > > (Updated Nov. 2, 2016, 5:44 a.m.) > > > Review request for Aurora, George Sirois, Joshua Cohen, Stephan Erb, and > Zameer Manji. > > > Bugs: AURORA-1014 > https://issue

Re: Review Request 52479: Resolve docker tags to concrete identifiers for DockerContainerizer

2016-10-11 Thread Joshua Cohen
insensitive. src/main/python/apache/aurora/client/docker/docker_client.py (line 111) <https://reviews.apache.org/r/52479/#comment221070> maybe use urlparse here? - Joshua Cohen On Oct. 11, 2016, 4:06 a.m., Santhosh Kumar Shanmugham wrote: > > --

Re: Review Request 52588: Enable per task volume mounts via scheduler API

2016-10-11 Thread Joshua Cohen
On Oct. 7, 2016, 7:19 p.m., Zameer Manji wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52588/ > --

Re: Review Request 52821: Fix the -enable_revocable_ram flag

2016-10-13 Thread Joshua Cohen
/ResourceType.java (line 215) <https://reviews.apache.org/r/52821/#comment221621> Is there a reason we can't apply the `get` call here rather than in the getter? - Joshua Cohen On Oct. 13, 2016, 9:27 a.m., Stephan Erb

Re: Review Request 52834: Introduce a --ip option to Thermos observer

2016-10-13 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52834/#review152566 --- Ship it! Ship It! - Joshua Cohen On Oct. 13, 2016, 3:32 p.m

Re: Review Request 52804: Adding an error message when the mesos_containerizer_path is not set correctly.

2016-10-13 Thread Joshua Cohen
that the file is executable as well. - Joshua Cohen On Oct. 13, 2016, 10:12 p.m., Justin Pinkul wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 52804: Adding an error message when the mesos_containerizer_path is not set correctly.

2016-10-12 Thread Joshua Cohen
ply, visit: > https://reviews.apache.org/r/52804/ > --- > > (Updated Oct. 12, 2016, 7:42 p.m.) > > > Review request for Aurora, Joshua Cohen and Zameer Manji. > > > Bugs: AURORA-1789 > https://issues.apache.org/jira/browse

Re: Review Request 52776: Blank out executor config in startJobUpdate log messages.

2016-10-12 Thread Joshua Cohen
/LoggingInterceptor.java (line 59) <https://reviews.apache.org/r/52776/#comment221340> Pull this out to a constant and reuse it between here and the `JobConfiguration` case? - Joshua Cohen On Oct. 12, 2016, 8:46 a.m., Stephan Erb

Re: Review Request 52790: Upgrade pystachio to 0.8.3

2016-10-12 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52790/#review152350 --- Ship it! Ship It! - Joshua Cohen On Oct. 12, 2016, 5:28 p.m

Re: Review Request 52915: Upgrade pants to the 1.3.0 dev series.

2016-10-17 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52915/#review152894 --- Ship it! Ship It! - Joshua Cohen On Oct. 15, 2016, 10:42

Re: Review Request 52921: In immutable Thrift structs, check identity before comparing fields.

2016-10-17 Thread Joshua Cohen
; } ? Curious if we're able to see a difference in benchmarks from this change? - Joshua Cohen On Oct. 16, 2016, 11:04 p.m., Stephan Erb wrote: > > --- > This is an automatically generated e-mail. To reply, visit

Re: Review Request 52884: Document how to create a custom CLI build

2016-10-14 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52884/#review152691 --- Ship it! Ship It! - Joshua Cohen On Oct. 14, 2016, 3:03 p.m

Re: Review Request 52479: Resolve docker tags to concrete identifiers for DockerContainerizer

2016-10-14 Thread Joshua Cohen
/#comment221805> Same here, re: BUILD src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh (line 542) <https://reviews.apache.org/r/52479/#comment221810> I'd say just use the binding helper as the default for the docker containerizer test case rather than re-running all the tests

Re: Review Request 54439: Add support for an mttu metric (median time to updated)

2016-12-07 Thread Joshua Cohen
ebated adding that when I was writing it, thanks for the nudge. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54439/#review158343 ---------

Re: Review Request 54439: Add support for an mttu metric (median time to updated)

2016-12-07 Thread Joshua Cohen
/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 6d0e9bc6a8040393875d4f0a88e8db9d6926a88b Diff: https://reviews.apache.org/r/54439/diff/ Testing --- ./gradlew build -Pq e2e tests. Thanks, Joshua Cohen

Re: Review Request 54459: Add message parameter to killTasks

2016-12-07 Thread Joshua Cohen
apache.org/r/54459/#comment229065> Fix indent. - Joshua Cohen On Dec. 7, 2016, 2:20 a.m., Cody Gibb wrote: > > --- > This is an automatically generated e-mail. To reply

Re: Review Request 54439: Add support for an mttu metric (median time to updated)

2016-12-08 Thread Joshua Cohen
ps://reviews.apache.org/r/54439/#review158363 ------- On Dec. 7, 2016, 5:50 p.m., Joshua Cohen wrote: > > --- > This is an automatically generated e-mail.

Re: Review Request 54255: Update to Mesos 1.1.0.

2016-12-07 Thread Joshua Cohen
to the review? RELEASE-NOTES.md (line 5) <https://reviews.apache.org/r/54255/#comment229120> 1.1.0 ;) - Joshua Cohen On Dec. 7, 2016, 11:04 a.m., Stephan Erb wrote: > > --- > This is an automatically generated e-mail.

Re: Review Request 53836: Get pants using the same thrift binary as gradle.

2016-12-07 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53836/#review158399 --- Ship it! Ship It! - Joshua Cohen On Dec. 7, 2016, 12:30 a.m

Re: Review Request 54774: Avoid double writing job updates to the Scheduler Snapshot

2016-12-15 Thread Joshua Cohen
/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java (line 128) <https://reviews.apache.org/r/54774/#comment230310> This `@Timed` annotation is on the wrong method now. - Joshua Cohen On Dec. 15, 2016, 6:16 p.m., David McLaughlin

Re: Review Request 54774: Avoid double writing job updates to the Scheduler Snapshot

2016-12-16 Thread Joshua Cohen
On Dec. 15, 2016, 8:04 p.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54774/ > --- > > (Updated Dec. 15, 2016, 8:04 p.m.) &g

Review Request 54428: Fix invalid logging that was causing pmd to NPE.

2016-12-06 Thread Joshua Cohen
--- Fix invalid logging that was causing pmd to NPE. Diffs - src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java ae7d019097f3e7049622623b48f3731198a08fb0 Diff: https://reviews.apache.org/r/54428/diff/ Testing --- Thanks, Joshua Cohen

Re: Review Request 54299: Extend warm-up time by `max_consecutive_failures` attempts.

2016-12-07 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54299/#review158490 --- Ship it! Ship It! - Joshua Cohen On Dec. 8, 2016, 12:15 a.m

Re: Review Request 54439: Add support for an mttu metric (median time to updated)

2016-12-09 Thread Joshua Cohen
ple active updates, seems like we could miss data > > points? Especially for small updates. > > Joshua Cohen wrote: > My thinking was that the vast majority of updates in the store will be > completed hours or days ago, so there's no need to consider them when > calc

Re: Review Request 54439: Add support for an mttu metric (median time to updated)

2016-12-09 Thread Joshua Cohen
ple active updates, seems like we could miss data > > points? Especially for small updates. > > Joshua Cohen wrote: > My thinking was that the vast majority of updates in the store will be > completed hours or days ago, so there's no need to consider them when > calc

Re: Review Request 54550: Specify python2.7 when extracting options from pants.ini during thrift bootstrap.

2016-12-08 Thread Joshua Cohen
`set -o xtrace`? TIL - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54550/#review158625 --- On Dec. 9, 20

Re: Review Request 54550: Specify python2.7 when extracting options from pants.ini during thrift bootstrap.

2016-12-08 Thread Joshua Cohen
a96b33e4564604f72d8fcf284f0cf1adea1866a9 Diff: https://reviews.apache.org/r/54550/diff/ Testing --- Thanks, Joshua Cohen

Review Request 54550: Specify python2.7 when extracting options from pants.ini during thrift bootstrap.

2016-12-08 Thread Joshua Cohen
--- Thanks, Joshua Cohen

Review Request 54569: Change back to thrift directory silently after executing pants command.

2016-12-08 Thread Joshua Cohen
--- Change back to thrift directory silently after executing pants command. Diffs - build-support/thrift/prepare_binary.sh e3b46aa1f36ed3eecc9bb7794073d18dd950eb1c Diff: https://reviews.apache.org/r/54569/diff/ Testing --- Ran this in vagrant. Thanks, Joshua Cohen

Re: Review Request 54567: Fixup prepare_binary.sh to work under modern bash.

2016-12-08 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54567/#review158638 --- Ship it! Ship It! - Joshua Cohen On Dec. 9, 2016, 3:58 a.m

Re: Review Request 55409: AURORA-1873 CuratorServiceGroupMonitor.LOG should use its own logger name

2017-01-11 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55409/#review161250 --- Ship it! Thanks for cleaning this up! - Joshua Cohen On Jan

Re: Review Request 55347: Ensure destination exists when mounting files into a filesystem image.

2017-01-09 Thread Joshua Cohen
reply, visit: https://reviews.apache.org/r/55347/#review160995 --- On Jan. 9, 2017, 5:38 p.m., Joshua Cohen wrote: > > --- > This is an automatically generated e-mail. To reply,

Re: Review Request 54967: AURORA-1856 Expose stats on deleted job updates in JobUpdateHistoryPruner

2017-01-11 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54967/#review161315 --- Ship it! Ship It! - Joshua Cohen On Dec. 22, 2016, 7:37 a.m

Re: Review Request 55347: Ensure destination exists when mounting files into a filesystem image.

2017-01-10 Thread Joshua Cohen
438eb0f03128d21b3201d0a843adebe09422c75e src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 46309371a33d26bdd467b5fa131634354be2a978 Diff: https://reviews.apache.org/r/55347/diff/ Testing --- ./build-support/jenkins/build.sh Thanks, Joshua Cohen

Re: Review Request 55347: Ensure destination exists when mounting files into a filesystem image.

2017-01-10 Thread Joshua Cohen
uler.conf#L43 > > list instead? Added to startup config and added e2e test as well. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55347/#review160933

Re: Review Request 55058: AURORA-1859 Expose stats on statically banned offers

2017-01-11 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55058/#review161319 --- Ship it! Ship It! - Joshua Cohen On Dec. 28, 2016, 6:25 p.m

Re: Review Request 55434: Reduce logging by ChainedStatusChecker and StatusManager when they're on the happy path.

2017-01-11 Thread Joshua Cohen
/python/apache/aurora/executor/common/status_checker.py f278825e58bba40c3b3ec735173705feb42bf165 src/main/python/apache/aurora/executor/status_manager.py 8b536a925e3f209c03b3eb44257096a0c0e497e0 Diff: https://reviews.apache.org/r/55434/diff/ Testing --- Thanks, Joshua Cohen

Review Request 55434: Reduce logging by ChainedStatusChecker and StatusManager when they're on the happy path.

2017-01-11 Thread Joshua Cohen
f278825e58bba40c3b3ec735173705feb42bf165 src/main/python/apache/aurora/executor/status_manager.py 8b536a925e3f209c03b3eb44257096a0c0e497e0 Diff: https://reviews.apache.org/r/55434/diff/ Testing --- Thanks, Joshua Cohen

Re: Review Request 54255: WIP: Update to Mesos 1.1.0.

2016-12-01 Thread Joshua Cohen
of curiosity, what was the build time after the changes? src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettingsLoader.java (line 94) <https://reviews.apache.org/r/54255/#comment228221> What's the reasoning for this change? - Joshua Cohen On Dec. 1, 2016

Re: Review Request 54107: changes to intercept and time mybatis invocations

2016-12-01 Thread Joshua Cohen
/InstrumentingInterceptorTest.java (line 40) <https://reviews.apache.org/r/54107/#comment228275> Is there a reason we need the power mock runner? - Joshua Cohen On Dec. 1, 2016, 8:39 p.m., Reza Motamedi wrote: > > --- > This is a

Re: Review Request 54107: changes to intercept and time mybatis invocations

2016-12-01 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54107/#review157662 --- @ReviewBot retry - Joshua Cohen On Dec. 1, 2016, 9:37 p.m

Re: Review Request 54288: Make leader elections resilient to ZK disconnections.

2016-12-02 Thread Joshua Cohen
> On Dec. 2, 2016, 3:58 p.m., Joshua Cohen wrote: > > src/test/java/org/apache/aurora/scheduler/discovery/CuratorSingletonServiceTest.java, > > lines 215-218 > > <https://reviews.apache.org/r/54288/diff/1/?file=1574553#file1574553line215> > > > > Shou

Re: Review Request 53836: Get pants using the same thrift binary as gradle.

2016-12-05 Thread Joshua Cohen
> On Nov. 17, 2016, 11:28 p.m., Joshua Cohen wrote: > > I'm seeing intermittent failures running the e2e tests with this patch > > applied. That's not necessarily alarming, as we've had reports of this on > > master as well, however the behavior in the event of these failu

Re: Review Request 54288: Make leader elections resilient to ZK disconnections.

2016-12-02 Thread Joshua Cohen
sleep for up to N seconds then `fail()`)? - Joshua Cohen On Dec. 2, 2016, 3:19 a.m., Zameer Manji wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.

Re: Review Request 54269: Improve scheduling throughput via logging changes.

2016-12-02 Thread Joshua Cohen
tps://reviews.apache.org/r/54269/#comment228400> Can you add a comment here explaining why we're not using `%class` and `%line`? They're definitely valuable when debugging, so I can imagine someone coming back in the future to change this. - Joshua Cohen On Dec. 1, 2016, 11:13 p.m., Zameer

Re: Review Request 54299: Extend warm-up time by `max_consecutive_failures` attempts.

2016-12-02 Thread Joshua Cohen
n the client, then the executor could only trigger the new behavior if the user opted in by setting `watch_secs` to 0 and `min_consecutive_successes` to non-zero. - Joshua Cohen On Dec. 2, 2016, 8:43 a.m., Santhosh Kumar Shan

Review Request 55347: Ensure destination exists when mounting files into a filesystem image.

2017-01-09 Thread Joshua Cohen
/jenkins/build.sh Thanks, Joshua Cohen

Re: Review Request 55105: AURORA-1870 Add finer grained timings to the Snapshot process

2017-01-04 Thread Joshua Cohen
/SnapshotStoreImpl.java (lines 520 - 532) <https://reviews.apache.org/r/55105/#comment231669> Can these be default methods on the interface rather than converting to an abstract class? - Joshua Cohen On Dec. 30, 2016, 9:18 p.m., Mehrdad Nurolahzade

Re: Review Request 55179: AURORA-1820 Reduce storage write lock contention by adopting Double-Checked Locking pattern in TimedOutTaskHandler

2017-01-04 Thread Joshua Cohen
-- > > (Updated Jan. 4, 2017, 5:16 p.m.) > > > Review request for Aurora, Joshua Cohen and Stephan Erb. > > > Bugs: AURORA-1820 > https://issues.apache.org/jira/browse/AURORA-1820 > > > Repository: aurora >

Re: Review Request 55179: AURORA-1820 Reduce storage write lock contention by adopting Double-Checked Locking pattern in TimedOutTaskHandler

2017-01-04 Thread Joshua Cohen
comments. - Joshua Cohen On Jan. 4, 2017, 5:16 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 54847: Remove ignored snapshot stats. Add high-level timings on storage start-up lifecycle.

2016-12-19 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54847/#review159616 --- Ship it! Ship It! - Joshua Cohen On Dec. 18, 2016, 11:51

Re: Review Request 54883: Move snapshots into a separate log

2016-12-27 Thread Joshua Cohen
> On Dec. 27, 2016, 10:50 p.m., Joshua Cohen wrote: > > Overall this looks good to me. I think some work would be required to > > productionize it (e.g. make it optional to start). Obviously want to vet > > this in a test cluster and we would need s

Re: Review Request 54883: Move snapshots into a separate log

2016-12-27 Thread Joshua Cohen
Manager.java (lines 92 - 94) <https://reviews.apache.org/r/54883/#comment231271> same here. src/main/java/org/apache/aurora/scheduler/storage/log/StreamManagerImpl.java (line 106) <https://reviews.apache.org/r/54883/#comment231272> +blank line before - Joshua Cohen On

Re: Review Request 54624: Expose stats on ZooKeeper connection state

2016-12-27 Thread Joshua Cohen
erence `Stats.STATS_PROVIDER` directly (if we ever bound another instance of `StatsProvider` in `AppModule` this class would not be aware of that, nor would we be able to use a mock stats provider for tests if we so desired. - Joshua Cohen On Dec. 20, 2016, 9:33 a.m., Jing Chen

Re: Review Request 54520: Revert BUILD changes in 0c177058.

2016-12-07 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54520/#review158497 --- Ship it! Ship It! - Joshua Cohen On Dec. 8, 2016, 4:39 a.m

Re: Review Request 58036: Ensure enum tables are complete afer a snapshot restore.

2017-03-29 Thread Joshua Cohen
> On March 29, 2017, 7:34 p.m., Joshua Cohen wrote: > > This may be an existing problem with the current impl as well, but what > > happens if we drop an enum value? Are we just assuming some other migration > > will have removed it (since presumably other tables wi

Re: Review Request 58036: Ensure enum tables are complete afer a snapshot restore.

2017-03-29 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58036/#review170462 --- Ship it! Ship It! - Joshua Cohen On March 29, 2017, 7:51

Re: Review Request 58036: Ensure enum tables are complete afer a snapshot restore.

2017-03-29 Thread Joshua Cohen
, but what happens if we drop an enum value? Are we just assuming some other migration will have removed it (since presumably other tables will need to be updated to have their FK relations fixed)? - Joshua Cohen On March 29, 2017, 7:21 p.m., Zameer Manji wrote

Re: Review Request 58467: Update to Mesos 1.2.0

2017-04-17 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58467/#review172085 --- Ship it! Ship It! - Joshua Cohen On April 15, 2017, 10:44

Re: Review Request 58524: Allow task to be in STARTING state during watch_secs.

2017-04-19 Thread Joshua Cohen
/updater/InstanceUpdater.java Lines 51 (patched) <https://reviews.apache.org/r/58524/#comment245450> 0L - Joshua Cohen On April 19, 2017, 9:07 a.m., Santhosh Kumar Shanmugham wrote: > > --- > This is an automatically g

Re: Review Request 58609: Switch Thermos runner to simple disk log layout

2017-04-21 Thread Joshua Cohen
assing this as a command line arg, we could just: from twitter.common.log.options import LogOptions LogOptions.set_simple(True) (We already do this in our internal fork, no idea why it's done there instead of the main project) - Joshua Cohen On April 21, 2017, 9:

Re: Review Request 58612: Improve cleanup hints in release and release-candidate scripts

2017-04-21 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58612/#review172656 --- Ship it! Ship It! - Joshua Cohen On April 21, 2017, 11:24

Re: Review Request 58609: Switch Thermos runner to simple disk log layout

2017-04-21 Thread Joshua Cohen
> On April 21, 2017, 2:37 p.m., Joshua Cohen wrote: > > src/main/python/apache/aurora/executor/thermos_task_runner.py > > Line 257 (original), 257 (patched) > > <https://reviews.apache.org/r/58609/diff/1/?file=1697172#file1697172line257> > > > >

Re: Review Request 57524: Support setting the rootfs on Mesos Containers.

2017-03-13 Thread Joshua Cohen
> On March 11, 2017, 12:28 p.m., Stephan Erb wrote: > > I need a little bit more context to understand what is going on here: > > > > * Do you plan to use this with Thermos or an alternative executor? Or both? > > * It seems like we don't need this for Thermos as we already create > >

Re: Review Request 58265: Add automatic browser tab open feature for aurora update start

2017-04-07 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58265/#review171356 --- Ship it! Ship It! - Joshua Cohen On April 7, 2017, 3:44 p.m

Re: Review Request 58651: Extend operator documentation

2017-04-24 Thread Joshua Cohen
t it's not entirely clear. - Joshua Cohen On April 23, 2017, 11:29 a.m., Stephan Erb wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.

Re: Review Request 59039: Add the ability to customize scheduling logic.

2017-05-12 Thread Joshua Cohen
> On May 12, 2017, 2:20 p.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/preemptor/PreemptorModule.java > > Line 130 (original), 174 (patched) > > > > > > Changes like these and others in this

Re: Review Request 59039: Add the ability to customize scheduling logic.

2017-05-12 Thread Joshua Cohen
, but since it's clearly labeled as alpha, caveat emptor~. - Joshua Cohen On May 11, 2017, 9:46 p.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 60306: Ensure Thermos is not overloaded by an unlimited number of lost processes

2017-06-21 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60306/#review178556 --- Ship it! Ship It! - Joshua Cohen On June 21, 2017, 9:37 p.m

Re: Review Request 60264: Add instructions to generate website for a release.

2017-06-21 Thread Joshua Cohen
) <https://reviews.apache.org/r/60264/#comment252521> maybe just make `instructions` the link, rather than `instructions README`, which reads somewhat awkwardly. - Joshua Cohen On June 21, 2017, 6:46 a.m., Santhosh Kumar Shanmugham

Re: Review Request 59495: Adding gpg key for santhk

2017-05-23 Thread Joshua Cohen
. - Joshua Cohen On May 23, 2017, 6:23 p.m., Santhosh Kumar Shanmugham wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 59495: Adding gpg key for santhk

2017-05-23 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59495/#review175854 --- Ship it! Ship It! - Joshua Cohen On May 23, 2017, 9:09 p.m

Re: Review Request 62857: Fix broken end-to-end tests

2017-10-10 Thread Joshua Cohen
> On Oct. 10, 2017, 2:29 p.m., Joshua Cohen wrote: > > I'd be shocked if they've been failing for over a year. We've had three > > releases since then and the release candidate verification script runs the > > end to end tests. > > > > If I had to haz

Re: Review Request 62857: Fix broken end-to-end tests

2017-10-10 Thread Joshua Cohen
working there. - Joshua Cohen On Oct. 10, 2017, 6:21 a.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 62857: Fix broken end-to-end tests

2017-10-10 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62857/#review187536 --- Ship it! Regardless... ship away! - Joshua Cohen On Oct. 10

Re: Review Request 62857: Fix broken end-to-end tests

2017-10-10 Thread Joshua Cohen
> On Oct. 10, 2017, 2:29 p.m., Joshua Cohen wrote: > > I'd be shocked if they've been failing for over a year. We've had three > > releases since then and the release candidate verification script runs the > > end to end tests. > > > > If I had to haz

Re: Review Request 62720: Implement Instance pages in React

2017-10-05 Thread Joshua Cohen
> On Oct. 4, 2017, 9:42 p.m., Stephan Erb wrote: > > Regarding the question nodejs vs js thrift: nodejs supports both binary > > (good) and compact (better) thrift protocols. This should reduce scheduler > > and client side serialization overhead and lead to a slightly more snappy > > UI. >

Re: Review Request 62958: Add URL handling for tab switching on Job page

2017-10-13 Thread Joshua Cohen
/URI), but I think it'd be ok to drop the query support in favor of a full url (or, if we're really concerned about bookmarks have a handler that redirects the query param form to the url form)? What do you think? - Joshua Cohen On Oct. 13, 2017, 12:10 a.m., David McLaughlin wrote

Re: Review Request 63052: Update list of Companies using Aurora.

2017-10-16 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63052/#review188252 --- Ship it! Ship It! - Joshua Cohen On Oct. 16, 2017, 11:18

Re: Review Request 60942: Remove task level resource fields from thrift interface and db

2017-08-30 Thread Joshua Cohen
r applied? I know generally we only support +/- 1 version, so we can get away with this if necessary, but I think it's more argument for leaving the add table in the above migration, and just dropping the migrate script? - Joshua Cohen On Aug. 24, 2017, 7:05 p.m., Nicolás Don

Re: Review Request 62135: HomePage implemented in Preact

2017-09-11 Thread Joshua Cohen
onents/Breadcrumbs.js ui/src/main/js/components/Home.js Line 4 (original), 3-5 (patched) <https://reviews.apache.org/r/62135/#comment261300> Just curious, why the move away from the ES6 style? - Joshua Cohen On Sept. 8, 2017, 11:40 p.m., David McLa

Re: Review Request 62607: Replace Preact and custom testing with React + Enzyme

2017-09-27 Thread Joshua Cohen
under MIT: https://facebook.github.io/react/blog/2017/09/25/react-v15.6.2.html - Joshua Cohen On Sept. 27, 2017, 3:04 p.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit

Re: Review Request 62607: Replace Preact and custom testing with React + Enzyme

2017-09-27 Thread Joshua Cohen
> On Sept. 27, 2017, 8:35 p.m., Kai Huang wrote: > > ui/src/main/js/components/Pagination.js > > Lines 93 (patched) > > > > > > Should this be extracted into renderer? So that we don't need to pass > > isTable

Re: Review Request 61864: Bootstrap the build pipeline for new Preact UI.

2017-08-24 Thread Joshua Cohen
e using jest? - Joshua Cohen On Aug. 24, 2017, 7:15 p.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://re

Re: Review Request 64288: Add a SQL persistence implementation

2017-12-04 Thread Joshua Cohen
apache/aurora/scheduler/storage/sql/SqlPersistence.java Lines 161 (patched) <https://reviews.apache.org/r/64288/#comment270979> Does MySQL support `NULLS FIRST`? I don't see any mention of it in their docs, and trying on a local MySQL instance (5.7.x) it doesn't seem to work? -

Re: Review Request 63685: RFC: Use new scheduler UI as landing page

2017-11-08 Thread Joshua Cohen
, but as you mention, the HTTP redirect is definitely not ideal. It should certainly be possible to serve up the actual scheduler UI by default without the need to redirect. Off the top of my head I'm not sure exactly what would need to be done though. - Joshua Cohen On Nov. 8, 2017, 10:32 p.m

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

2017-10-31 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63436/#review189756 --- Ship it! Ship It! - Joshua Cohen On Oct. 31, 2017, 8:15 p.m

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

2017-10-30 Thread Joshua Cohen
ply, visit: > https://reviews.apache.org/r/63436/ > --- > > (Updated Oct. 31, 2017, 1:58 a.m.) > > > Review request for Aurora, David McLaughlin, Joshua Cohen, and Kai Huang. > > > Repository: aurora > &g

Re: Review Request 60942: Remove task level resource fields from thrift interface and db

2017-10-19 Thread Joshua Cohen
g/r/60942/#review188555 --- On Aug. 24, 2017, 7:05 p.m., Nicolás Donatucci wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60942/

Re: Review Request 63383: Suppress multiline logging from mesos callbacks

2017-10-27 Thread Joshua Cohen
> On Oct. 28, 2017, 12:08 a.m., Jordan Ly wrote: > > src/main/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandler.java > > Lines 303-305 (patched) > > > > > > Does Mesos send any other important information

<    1   2   3   4   5   6