Re: Review Request 59698: Allow custom OfferManager ordering to be injected via Guice modules

2017-06-01 Thread David McLaughlin
s://reviews.apache.org/r/59698/#review176658 ------- On June 1, 2017, 12:14 a.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59698/ > --

Re: Review Request 59698: Allow custom OfferManager ordering to be injected via Guice modules

2017-06-01 Thread David McLaughlin
s is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59698/#review176660 ------- On June 1, 2017, 12:14 a.m., David McLaughlin wrote: > > --- > Thi

Re: Review Request 59699: Improve task history pruning by batch deleting tasks

2017-06-01 Thread David McLaughlin
/StateManagerImpl.java Lines 389-391 (original), 374-376 (patched) <https://reviews.apache.org/r/59699/#comment250086> We probably don't even need the separate events. We could just have: eventSink.post(createDeleteEvent(taskStore, taskIds)); - David McLaughlin On June 1, 2017, 9:1

Re: Review Request 59703: Use async HTTP for Web Hooks.

2017-06-01 Thread David McLaughlin
y, visit: https://reviews.apache.org/r/59703/#review17 ------- On June 1, 2017, 6:33 a.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail.

Re: Review Request 59699: Improve task history pruning by batch deleting tasks

2017-06-01 Thread David McLaughlin
> On June 1, 2017, 9:22 p.m., David McLaughlin wrote: > > src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java > > Lines 389-391 (original), 374-376 (patched) > > <https://reviews.apache.org/r/59699/diff/1-2/?file=1736080#file1736080line391> > &g

Re: Review Request 59699: Improve task history pruning by batch deleting tasks

2017-06-01 Thread David McLaughlin
logic was there in the previous patch too. This would ensure consistency between taskStore.deleteTasks and the taskIds we broadcast as deleted. - David McLaughlin On June 1, 2017, 10:41 p.m., Kai Huang wrote: > > --- > This

Re: Review Request 59699: Improve task history pruning by batch deleting tasks

2017-06-01 Thread David McLaughlin
> On June 1, 2017, 10:46 p.m., David McLaughlin wrote: > > src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java > > Line 376 (original), 377 (patched) > > <https://reviews.apache.org/r/59699/diff/3/?file=1739138#file1739138line379> > > > >

Re: Review Request 59733: Adding Configurable Wait Period for Graceful Shutdowns

2017-06-01 Thread David McLaughlin
37 (patched) <https://reviews.apache.org/r/59733/#comment250135> Will this break for tasks that were deployed before the client was updated? Should we also inject a default here? - David McLaughlin On June 1, 2017, 11:48 p.m., Jordan Ly

Re: Review Request 59699: Improve task history pruning by batch deleting tasks

2017-06-01 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59699/#review176716 --- Ship it! Ship It! - David McLaughlin On June 2, 2017, 12:01

Re: Review Request 59733: Adding Configurable Wait Period for Graceful Shutdowns

2017-06-02 Thread David McLaughlin
- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59733/ > ------- > > (Updated June 1, 2017, 11:48 p.m.) > > > Review request for Aurora, David McLaughlin, Santhosh Kumar

Re: Review Request 59733: Adding Configurable Wait Period for Graceful Shutdowns

2017-06-02 Thread David McLaughlin
sure tasks will actually termiante. Please add an e2e test here to > > confirm/deny. > > David McLaughlin wrote: > For (3), this is exactly what STOP_TIMEOUT in the executor is for. > > The issue for STOP_TIMEOUT is it is Thermos-specific and we support > multiple

Re: Review Request 59698: Allow custom OfferManager ordering to be injected via Guice modules

2017-06-02 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59698/#review176817 --- Ping. This still needs committer Ship-Its. - David McLaughlin

Re: Review Request 59699: Improve task history pruning by batch deleting tasks

2017-06-02 Thread David McLaughlin
> On June 2, 2017, 12:35 a.m., David McLaughlin wrote: > > Ship It! > > Kai Huang wrote: > It looks like the deleteTasks() method could be even simpler: perhaps we > don't need to move taskStore.deleteTasks out of the deleteEvent factory > method? The only tri

Re: Review Request 59733: Adding Configurable Wait Period for Graceful Shutdowns

2017-06-02 Thread David McLaughlin
sure tasks will actually termiante. Please add an e2e test here to > > confirm/deny. > > David McLaughlin wrote: > For (3), this is exactly what STOP_TIMEOUT in the executor is for. > > The issue for STOP_TIMEOUT is it is Thermos-specific and we support > multipl

Re: Review Request 59699: Improve task history pruning by batch deleting tasks

2017-06-02 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59699/#review176845 --- This has been merged to master. - David McLaughlin On June 2

Re: Review Request 59640: Prioritize adding instances over updating instances during an update

2017-06-02 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59640/#review176852 --- This has been merged to master. - David McLaughlin On June 1

Re: Review Request 59699: Improve task history pruning by batch deleting tasks

2017-06-05 Thread David McLaughlin
> On June 2, 2017, 8:42 p.m., David McLaughlin wrote: > > This has been merged to master. > > Stephan Erb wrote: > Did you leave this open on purpose or so that the other reviewers still > have a chance to see it? Closing it now. No, I don't have permission to

Re: Review Request 59703: Use async HTTP for Web Hooks.

2017-06-06 Thread David McLaughlin
ttps://reviews.apache.org/r/59703/#review176961 ------- On June 1, 2017, 6:33 a.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mai

Re: Review Request 59703: Use async HTTP for Web Hooks.

2017-06-06 Thread David McLaughlin
quest handling thread. So we will just > > have one outgoing request at the time, correct? > > David McLaughlin wrote: > > So we will just have one outgoing request at the time, correct? > > Quite the contrary. In practice we will have thousands of requests in

Re: Review Request 59703: Use async HTTP for Web Hooks.

2017-06-06 Thread David McLaughlin
shipping. - David McLaughlin On June 1, 2017, 6:33 a.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 59703: Use async HTTP for Web Hooks.

2017-06-06 Thread David McLaughlin
quest handling thread. So we will just > > have one outgoing request at the time, correct? > > David McLaughlin wrote: > > So we will just have one outgoing request at the time, correct? > > Quite the contrary. In practice we will have thousands of requests in

Re: Review Request 59853: Process rescinds in the same thread pool as offers.

2017-06-06 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59853/#review177086 --- Ship it! Ship It! - David McLaughlin On June 6, 2017, 7:42

Re: Review Request 59733: Adding Configurable Wait Period for Graceful Shutdowns

2017-06-06 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59733/#review177090 --- Ship it! Ship It! - David McLaughlin On June 6, 2017, 8:02

Re: Review Request 59703: Use async HTTP for Web Hooks.

2017-06-07 Thread David McLaughlin
> On June 6, 2017, 5:01 p.m., David McLaughlin wrote: > > Just a heads up: we are scale-testing this internally before shipping. This change has been verified in a scale-test environment (handling ~50k task state changes per minute).

Re: Review Request 59703: Use async HTTP for Web Hooks.

2017-06-07 Thread David McLaughlin
quest handling thread. So we will just > > have one outgoing request at the time, correct? > > David McLaughlin wrote: > > So we will just have one outgoing request at the time, correct? > > Quite the contrary. In practice we will have thousands of requests in

Re: Review Request 59883: Includes: - Adding eclipse generated files to .gitignore - Support specifying a custom ShiroCredentialsMatcher

2017-06-08 Thread David McLaughlin
ng to achieve? - David McLaughlin On June 8, 2017, 7:25 a.m., Ruben D. Porras wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.a

Re: Review Request 59940: Add a whitelist for TaskStateChange events in Webhook.

2017-06-08 Thread David McLaughlin
ve emulated here. Suggest refactoring the module with a constructor that accepts a WebhookInfo in order to achieve the desired testability. - David McLaughlin On June 9, 2017, 5:13 a.m., Kai Huang wrote: > > --- > This is an automatic

Re: Review Request 59883: Includes: - Adding eclipse generated files to .gitignore - Support specifying a custom ShiroCredentialsMatcher

2017-06-09 Thread David McLaughlin
> On June 8, 2017, 7:48 p.m., David McLaughlin wrote: > > Can you give a higher level description of what you're trying to achieve? > > Ruben D. Porras wrote: > Allows to use a custom CredentialsMatchers... > > - using the command line argument **shi

Re: Review Request 59883: Includes: - Adding eclipse generated files to .gitignore - Support specifying a custom ShiroCredentialsMatcher

2017-06-12 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59883/#review177655 --- Ship it! Ship It! - David McLaughlin On June 8, 2017, 7:25

Re: Review Request 59733: Adding Configurable Wait Period for Graceful Shutdowns

2017-06-12 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59733/#review177659 --- Ship it! Ship It! - David McLaughlin On June 12, 2017, 6:37

Re: Review Request 59940: Add a whitelist for TaskStateChange events in Webhook.

2017-06-13 Thread David McLaughlin
/apache/aurora/scheduler/events/Webhook.java Lines 54 (patched) <https://reviews.apache.org/r/59940/#comment251453> This seems clearer: webhookInfo.getWhitelistedStatuses().isPresent() && webhookInfo.getWhitelistedStatuses().get().contains(status) - David McLaughli

Re: Review Request 59940: Add a whitelist for TaskStateChange events in Webhook.

2017-06-13 Thread David McLaughlin
> On June 13, 2017, 2:24 p.m., David McLaughlin wrote: > > src/main/java/org/apache/aurora/scheduler/events/Webhook.java > > Lines 54 (patched) > > <https://reviews.apache.org/r/59940/diff/2/?file=1749519#file1749519line54> >

Re: Review Request 60133: Update h2 to 1.4.196

2017-06-15 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60133/#review178058 --- Ship it! Ship It! - David McLaughlin On June 15, 2017, 10

Re: Review Request 59703: Use async HTTP for Web Hooks.

2017-06-16 Thread David McLaughlin
> On June 6, 2017, 5:01 p.m., David McLaughlin wrote: > > Just a heads up: we are scale-testing this internally before shipping. > > David McLaughlin wrote: > This change has been verified in a scale-test environment (handling ~50k > task state changes per minute). I

Re: Review Request 60173: Allow custom Thrift method interceptors to be injected via Guice modules

2017-06-20 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60173/#review178424 --- Ship it! Ship It! - David McLaughlin On June 19, 2017, 6:03

Re: Review Request 60350: Add missing stats in MesosCallbackHandler

2017-06-22 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60350/#review178709 --- Ship it! Ship It! - David McLaughlin On June 22, 2017, 12

Re: Review Request 60354: Observer task page to load consumption info from history

2017-06-22 Thread David McLaughlin
/main/python/apache/thermos/monitoring/resource.py Line 160 (original), 166 (patched) <https://reviews.apache.org/r/60354/#comment252861> I think better variable names would help with reading this code. - David McLaughlin On June 22, 2017, 3:28 p.m., Reza Motamedi

Re: Review Request 60437: Add timing metrics in MesosCallbackHandler for backward compatibility.

2017-06-26 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60437/#review178912 --- Ship it! Ship It! - David McLaughlin On June 26, 2017, 5:37

Re: Review Request 59853: Process rescinds in the same thread pool as offers.

2017-06-28 Thread David McLaughlin
deployed it to production. We are seeing more tasks lost due to being launched in invalid offers. I think we should revert this change until we root cause it. - David McLaughlin On June 6, 2017, 8:56 p.m., Zameer Manji wrote: > > ---

Re: Review Request 59853: Process rescinds in the same thread pool as offers.

2017-06-28 Thread David McLaughlin
> On June 28, 2017, 5:22 p.m., David McLaughlin wrote: > > This change appears to have caused adverse behavior when we deployed it to > > production. We are seeing more tasks lost due to being launched in invalid > > offers. > > > > I think we should revert t

Re: Review Request 60787: Added PayPal to the list of companies using Aurora

2017-07-11 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60787/#review180272 --- Ship it! Ship It! - David McLaughlin On July 12, 2017, 1:36

Re: Review Request 60714: aurora job restart request should be idempotent and retryable

2017-07-15 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60714/#review180637 --- Ship it! Ship It! - David McLaughlin On July 7, 2017, 11:52

Re: Review Request 61238: Fix preemption ordering when tasks contain different ResourceTypes

2017-07-28 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61238/#review181754 --- Ship it! Ship It! - David McLaughlin On July 29, 2017, 2:30

Re: Review Request 61249: Cron timezone should be configurable per job

2017-08-02 Thread David McLaughlin
tch landed, and below it will default to "GMT", possibly conflicting with the scheduler default. So you can either do a null check here and call predictNextRun when it's null, or do a storage backfill and add timezone to all existing crons. - David McLaughlin

Re: Review Request 61249: Cron timezone should be configurable per job

2017-08-02 Thread David McLaughlin
> On Aug. 2, 2017, 8:38 p.m., David McLaughlin wrote: > > src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java > > Lines 266 (patched) > > <https://reviews.apache.org/r/61249/diff/1/?file=1785805#file1785805line266> > > > > Th

Re: Review Request 61249: Cron timezone should be configurable per job

2017-08-03 Thread David McLaughlin
> On Aug. 2, 2017, 8:38 p.m., David McLaughlin wrote: > > api/src/main/thrift/org/apache/aurora/gen/api.thrift > > Lines 328 (patched) > > <https://reviews.apache.org/r/61249/diff/1/?file=1785797#file1785797line328> > > > > Needs to be marked option

Re: Review Request 61249: Cron timezone should be configurable per job

2017-08-03 Thread David McLaughlin
> On Aug. 2, 2017, 8:38 p.m., David McLaughlin wrote: > > src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java > > Lines 266 (patched) > > <https://reviews.apache.org/r/61249/diff/1/?file=1785805#file1785805line266> > > > > Th

Re: Review Request 61804: Fix race condition where rescinds are received but not processed before offer is accepted

2017-08-22 Thread David McLaughlin
On Aug. 22, 2017, 5:05 p.m., Jordan Ly wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61804/ > ------- > >

Re: Review Request 61804: Fix race condition where rescinds are received but not processed before offer is accepted

2017-08-22 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61804/#review183514 --- Ship it! Ship It! - David McLaughlin On Aug. 22, 2017, 5:05

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

2017-08-23 Thread David McLaughlin
t.js PRE-CREATION ui/src/main/js/index.js PRE-CREATION ui/webpack.config.js PRE-CREATION Diff: https://reviews.apache.org/r/61864/diff/1/ Testing --- The Scheduler is running in my Vagrant image with the Hello World message served under /beta. Thanks, David McLaughlin

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

2017-08-23 Thread David McLaughlin
Testing --- The Scheduler is running in my Vagrant image with the Hello World message served under /beta. Thanks, David McLaughlin

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

2017-08-24 Thread David McLaughlin
/home/vagrant/aurora/ui/npm-debug.log > > :ui:install FAILED > > > > FAILURE: Build failed with an exception. > > > > * What went wrong: > > Execution failed for task ':ui:install'. > > > Process 'command > >

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

2017-08-24 Thread David McLaughlin
Testing --- The Scheduler is running in my Vagrant image with the Hello World message served under /beta. Thanks, David McLaughlin

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

2017-08-24 Thread David McLaughlin
in at global scope? Removed. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61864/#review183766 --- On Aug. 24,

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

2017-08-24 Thread David McLaughlin
; > ./config/donation-message.js'. > > npm ERR! Make sure you have the latest version of node.js and npm > > installed. > > npm ERR! If you do, this is most likely a problem with the preact > > package, > > npm ERR! not with npm itself. > >

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

2017-08-24 Thread David McLaughlin
che.org/r/61864/diff/3-4/ Testing --- The Scheduler is running in my Vagrant image with the Hello World message served under /beta. Thanks, David McLaughlin

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

2017-08-24 Thread David McLaughlin
3110#file1803110line9> > > > > Are we using jest? Removed. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61864/#review183788 ---------

Review Request 62135: HomePage implemented in Preact

2017-09-08 Thread David McLaughlin
/1/ Testing --- ./gradlew ui:test Running in vagrant (screenshots attached). File Attachments preview https://reviews.apache.org/media/uploaded/files/2017/09/08/011f99fb-a14e-4547-b90d-a5a1909c737c__Screen_Shot_2017-09-08_at_3.58.48_PM.png Thanks, David McLaughlin

Re: Review Request 62135: HomePage implemented in Preact

2017-09-11 Thread David McLaughlin
-- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62135/#review185062 ------- On Sept. 8, 2017, 11:40 p.m., David McLaughlin wrote: > > -

Re: Review Request 62135: HomePage implemented in Preact

2017-09-11 Thread David McLaughlin
ort default on a separate line? - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62135/#review185082 --- On S

Re: Review Request 62135: HomePage implemented in Preact

2017-09-13 Thread David McLaughlin
://reviews.apache.org/media/uploaded/files/2017/09/08/011f99fb-a14e-4547-b90d-a5a1909c737c__Screen_Shot_2017-09-08_at_3.58.48_PM.png Thanks, David McLaughlin

Re: Review Request 62135: HomePage implemented in Preact

2017-09-13 Thread David McLaughlin
reply, visit: https://reviews.apache.org/r/62135/#review185096 ------- On Sept. 13, 2017, 5:44 p.m., David McLaughlin wrote: > > --- > This is an automatically

Re: Review Request 62135: HomePage implemented in Preact

2017-09-13 Thread David McLaughlin
reply, visit: https://reviews.apache.org/r/62135/#review185314 --- On Sept. 13, 2017, 5:44 p.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. T

Re: Review Request 62135: HomePage implemented in Preact

2017-09-13 Thread David McLaughlin
/media/uploaded/files/2017/09/08/011f99fb-a14e-4547-b90d-a5a1909c737c__Screen_Shot_2017-09-08_at_3.58.48_PM.png Thanks, David McLaughlin

Re: Review Request 62135: HomePage implemented in Preact

2017-09-13 Thread David McLaughlin
----- On Sept. 13, 2017, 6:07 p.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62135/ >

Re: Review Request 62135: HomePage implemented in Preact

2017-09-13 Thread David McLaughlin
> On Sept. 13, 2017, 6:13 p.m., Aurora ReviewBot wrote: > > This patch does not apply cleanly against master (3ec0430), do you need to > > rebase? > > > > I will refresh this build result if you post a review containing > > "@ReviewBot retry" >

Re: Review Request 62135: HomePage implemented in Preact

2017-09-13 Thread David McLaughlin
/media/uploaded/files/2017/09/08/011f99fb-a14e-4547-b90d-a5a1909c737c__Screen_Shot_2017-09-08_at_3.58.48_PM.png Thanks, David McLaughlin

Re: Review Request 61249: Cron timezone should be configurable per job

2017-09-20 Thread David McLaughlin
an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61249/ > ------- > > (Updated Sept. 12, 2017, 8:38 a.m.) > > > Review request for Aurora, David McLaughlin, Joshua Cohen, and Stephan Erb. > > > Bu

Review Request 62451: Implement Role and Environment pages in Preact.

2017-09-21 Thread David McLaughlin
File Attachments Screenshot https://reviews.apache.org/media/uploaded/files/2017/09/21/684d8630-a9b4-4fbf-8955-a7fc118afcce__Screen_Shot_2017-09-21_at_4.11.05_PM.png Thanks, David McLaughlin

Re: Review Request 62517: Update to gradle 4.2

2017-09-22 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62517/#review186033 --- Ship it! Ship It! - David McLaughlin On Sept. 22, 2017, 10

Re: Review Request 62558: Restore scheduler benchmarks to working order

2017-09-26 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62558/#review186276 --- Ship it! Ship It! - David McLaughlin On Sept. 26, 2017, 6

Re: Review Request 62601: Remove the rewriteConfigs thrift method

2017-09-26 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62601/#review186374 --- Ship it! Ship It! - David McLaughlin On Sept. 27, 2017, 12

Re: Review Request 62601: Remove the rewriteConfigs thrift method

2017-09-26 Thread David McLaughlin
> On Sept. 27, 2017, 5:21 a.m., David McLaughlin wrote: > > Ship It! +1 from Twitter for skipping the deprecation cycle - we do not use this. - David --- This is an automatically generated e-mail. To reply, vis

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

2017-09-27 Thread David McLaughlin
--- Unfortunately the Gradle 4.2 upgrade seems to have broken running builds from gradle (will attempt to fix in another ticket)... So I had to manually verify with: $ npm run lint $ npm test I also tested the UI in my Vagrant image. Thanks, David McLaughlin

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

2017-09-27 Thread David McLaughlin
marked as UP-TO-DATE even after changing the sources. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62607/#review186421 ---------

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

2017-09-27 Thread David McLaughlin
org/r/62607/#review186425 --- On Sept. 27, 2017, 3:04 p.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: &g

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

2017-09-27 Thread David McLaughlin
verify with: $ npm run lint $ npm test I also tested the UI in my Vagrant image. Thanks, David McLaughlin

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

2017-09-27 Thread David McLaughlin
tps://reviews.apache.org/r/62620/). I'll be curious what's > > going on if that fix your setup as well. > > David McLaughlin wrote: > To be clear: it doesn't break the build. But ./gradlew ui:test and > ./gradlew ui:lint were giving false positives and then wer

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

2017-09-27 Thread David McLaughlin
my Vagrant image. Thanks, David McLaughlin

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

2017-09-27 Thread David McLaughlin
-- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62607/#review186469 --- On Sept. 27, 2017, 5:51 p.m., David McLaughlin wrote: > > -

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

2017-09-27 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62607/#review186472 --- On Sept. 27, 2017, 5:51 p.m., David McLaughlin wrote: > > ---

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

2017-09-27 Thread David McLaughlin
/webpack.config.js e7cd672df62006bd5c5b60a6ba0903e16a767d13 Diff: https://reviews.apache.org/r/62607/diff/4/ Changes: https://reviews.apache.org/r/62607/diff/3-4/ Testing --- $ ./gradlew ui:lint $ ./gradlew ui:test I also tested the UI in my Vagrant image. Thanks, David McLaughlin

Re: Review Request 62451: Implement Role and Environment pages in Preact.

2017-09-27 Thread David McLaughlin
47cff691058972aebc5f79f22955cf2448ce48c > > > Diff: https://reviews.apache.org/r/62451/diff/1/ > > > Testing > --- > > ./gradlew ui:test > ./gradlew ui:lint > > See screenshot for local development with some Twitter data. > > > File Attachments > > > Screenshot > > https://reviews.apache.org/media/uploaded/files/2017/09/21/684d8630-a9b4-4fbf-8955-a7fc118afcce__Screen_Shot_2017-09-21_at_4.11.05_PM.png > > > Thanks, > > David McLaughlin > >

Re: Review Request 62451: Implement Role and Environment pages in Preact.

2017-09-27 Thread David McLaughlin
unblock others from contributing their ideas. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62451/#review186142 -------

Re: Review Request 62451: Implement Role and Environment pages in Preact.

2017-09-27 Thread David McLaughlin
-- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62451/#review186347 ------- On Sept. 21, 2017, 11:17 p.m., David McLaughlin wrote: > > ---

Re: Review Request 62451: Implement Role and Environment pages in Preact.

2017-09-27 Thread David McLaughlin
. File Attachments Screenshot https://reviews.apache.org/media/uploaded/files/2017/09/21/684d8630-a9b4-4fbf-8955-a7fc118afcce__Screen_Shot_2017-09-21_at_4.11.05_PM.png Thanks, David McLaughlin

Re: Review Request 62451: Implement Role and Environment pages in Preact.

2017-09-27 Thread David McLaughlin
local development with some Twitter data. File Attachments Screenshot https://reviews.apache.org/media/uploaded/files/2017/09/21/684d8630-a9b4-4fbf-8955-a7fc118afcce__Screen_Shot_2017-09-21_at_4.11.05_PM.png Thanks, David McLaughlin

Re: Review Request 62451: Implement Role and Environment pages in Preact.

2017-09-27 Thread David McLaughlin
ave customer tier configuration. Should > > we make this read that configuration? > > > > http://aurora.apache.org/documentation/latest/features/multitenancy/ > > David McLaughlin wrote: > Maybe in a future patch (although I'd question the value tbh). I&#

Re: Review Request 62652: Remove legacy commons ZK code

2017-09-27 Thread David McLaughlin
curator in production: https://issues.apache.org/jira/browse/AURORA-1840 - David McLaughlin On Sept. 28, 2017, 3:14 a.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.

Re: Review Request 62623: Use a simpler command line argument system

2017-09-29 Thread David McLaughlin
-- > > (Updated Sept. 28, 2017, 4:22 a.m.) > > > Review request for Aurora, David McLaughlin and John Sirois. > > > Repository: aurora > > > Description > --- > > **NOTE: this patch is not ready to commit, but is ready for initial > dis

Re: Review Request 62700: Convert Webhook to AbstractIdleService, use async HTTP client

2017-09-29 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62700/#review186765 --- Ship it! Ship It! - David McLaughlin On Sept. 30, 2017, 12

Re: Review Request 62623: Use a simpler command line argument system

2017-10-01 Thread David McLaughlin
> On Sept. 29, 2017, 6:17 p.m., David McLaughlin wrote: > > I'm also +1 to the spirit of the patch. One requirement to keep in mind > > beyond grouping arguments into logical modules is that we also need to make > > sure we keep the ability to add custom C

Re: Review Request 62692: Move job environment validation to the scheduler.

2017-10-01 Thread David McLaughlin
! Ship It contingent on a green ReviewBot result. - David McLaughlin On Sept. 30, 2017, 3:04 p.m., Mauricio Garavaglia wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 62716: Replace auto-generated forwarding code with manual implementations

2017-10-01 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62716/#review186799 --- Ship it! Ship It! - David McLaughlin On Sept. 30, 2017, 5

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

2017-10-01 Thread David McLaughlin
e don't need to pass > > isTable flags? > > David McLaughlin wrote: > I added a comment that describes why the user must provide this. You > cannot return a list of elements - you have to return a single container. So > Pagination needs to know what to wrap your i

Re: Review Request 62451: Implement Role and Environment pages in Preact.

2017-10-01 Thread David McLaughlin
See screenshot for local development with some Twitter data. File Attachments Screenshot https://reviews.apache.org/media/uploaded/files/2017/09/21/684d8630-a9b4-4fbf-8955-a7fc118afcce__Screen_Shot_2017-09-21_at_4.11.05_PM.png Thanks, David McLaughlin

Re: Review Request 62451: Implement Role and Environment pages in Preact.

2017-10-03 Thread David McLaughlin
pment with some Twitter data. File Attachments Screenshot https://reviews.apache.org/media/uploaded/files/2017/09/21/684d8630-a9b4-4fbf-8955-a7fc118afcce__Screen_Shot_2017-09-21_at_4.11.05_PM.png Thanks, David McLaughlin

Re: Review Request 62623: Use a simpler command line argument system

2017-10-03 Thread David McLaughlin
> On Sept. 29, 2017, 6:17 p.m., David McLaughlin wrote: > > I'm also +1 to the spirit of the patch. One requirement to keep in mind > > beyond grouping arguments into logical modules is that we also need to make > > sure we keep the ability to add custom C

Review Request 62720: Implement Instance pages in React

2017-10-03 Thread David McLaughlin
https://reviews.apache.org/media/uploaded/files/2017/10/04/f5daaf35-4e4b-4106-b7ce-2773b97d2277__Screen_Shot_2017-10-03_at_10.28.18_PM.png Tooltip https://reviews.apache.org/media/uploaded/files/2017/10/04/0fe652c3-48a3-4292-8179-7d09bfbcc1e4__Screen_Shot_2017-10-03_at_10.28.33_PM.png Thanks, David McLaughlin

Re: Review Request 62720: Implement Instance pages in React

2017-10-03 Thread David McLaughlin
anded Finished Task https://reviews.apache.org/media/uploaded/files/2017/10/04/f5daaf35-4e4b-4106-b7ce-2773b97d2277__Screen_Shot_2017-10-03_at_10.28.18_PM.png Tooltip https://reviews.apache.org/media/uploaded/files/2017/10/04/0fe652c3-48a3-4292-8179-7d09bfbcc1e4__Screen_Shot_2017-10-03_at_10.28.33_PM.png Thanks, David McLaughlin

<    1   2   3   4   5   6   >