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/
> --
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
/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
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.
> 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
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
> 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>
> >
> >
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
---
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
-
> 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
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
---
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
> 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
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
---
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
---
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
> 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
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
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
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
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
---
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
---
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
> 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).
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
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
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
> 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
---
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
---
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
/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
> 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>
>
---
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
> 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
---
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
---
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
/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
---
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
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:
>
> ---
> 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
---
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
---
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
---
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
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
> 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
> 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
> 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
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/
> -------
>
>
---
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
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
Testing
---
The Scheduler is running in my Vagrant image with the Hello World message
served under /beta.
Thanks,
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
> >
Testing
---
The Scheduler is running in my Vagrant image with the Hello World message
served under /beta.
Thanks,
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,
; > ./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.
> >
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
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
---------
/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
--
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:
>
> -
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
://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
reply, visit:
https://reviews.apache.org/r/62135/#review185096
-------
On Sept. 13, 2017, 5:44 p.m., David McLaughlin wrote:
>
> ---
> This is an automatically
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
/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
-----
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/
>
> 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"
>
/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
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
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
---
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
---
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
---
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
> 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
---
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
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
---------
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
verify
with:
$ npm run lint
$ npm test
I also tested the UI in my Vagrant image.
Thanks,
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
my Vagrant image.
Thanks,
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:
>
> -
---
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:
>
> ---
/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
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
>
>
unblock others from contributing their ideas.
- David
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62451/#review186142
-------
--
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:
>
> ---
.
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
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
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
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.
--
>
> (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
---
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
> 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
!
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
---
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
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
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
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
> 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
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
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
201 - 300 of 574 matches
Mail list logo