You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Bill Farner <wf...@apache.org> on 2015/04/17 00:00:17 UTC
Review Request 33279: Add SQL tables needed for a datbase-backed task
store.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33279/
-----------------------------------------------------------
Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
Bugs: AURORA-556
https://issues.apache.org/jira/browse/AURORA-556
Repository: aurora
Description
-------
Add SQL tables needed for a datbase-backed task store.
Diffs
-----
src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql ed73ef7a25537f145910dba7a1985805b3979173
Diff: https://reviews.apache.org/r/33279/diff/
Testing
-------
Thanks,
Bill Farner
Re: Review Request 33279: Add SQL tables needed for a datbase-backed
task store.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33279/#review80387
-----------------------------------------------------------
Master (b18dc44) is green with this patch.
./build-support/jenkins/build.sh
However, it appears that it might lack test coverage.
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On April 16, 2015, 10 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33279/
> -----------------------------------------------------------
>
> (Updated April 16, 2015, 10 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
>
>
> Bugs: AURORA-556
> https://issues.apache.org/jira/browse/AURORA-556
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Add SQL tables needed for a datbase-backed task store.
>
>
> Diffs
> -----
>
> src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql ed73ef7a25537f145910dba7a1985805b3979173
>
> Diff: https://reviews.apache.org/r/33279/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 33279: Add SQL tables needed for a datbase-backed
task store.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On April 20, 2015, 5:50 p.m., Maxim Khutornenko wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, line 200
> > <https://reviews.apache.org/r/33279/diff/1/?file=932067#file932067line200>
> >
> > Isn't this too restrictive? Currently, we can still have something like 'rack':'limit:5' and 'rack':'abc'. This constraint will now allow configs like that any longer.
s/now/not
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33279/#review80750
-----------------------------------------------------------
On April 16, 2015, 10 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33279/
> -----------------------------------------------------------
>
> (Updated April 16, 2015, 10 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
>
>
> Bugs: AURORA-556
> https://issues.apache.org/jira/browse/AURORA-556
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Add SQL tables needed for a datbase-backed task store.
>
>
> Diffs
> -----
>
> src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql ed73ef7a25537f145910dba7a1985805b3979173
>
> Diff: https://reviews.apache.org/r/33279/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 33279: Add SQL tables needed for a datbase-backed
task store.
Posted by Bill Farner <wf...@apache.org>.
> On April 20, 2015, 5:50 p.m., Maxim Khutornenko wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, line 179
> > <https://reviews.apache.org/r/33279/diff/1/?file=932067#file932067line179>
> >
> > What are your plans on retaining the "user" part of the Identity struct (AURORA-749)? If you are planning to address it later a follow up TODO here would be nice to avoid dropping this issue off the radar.
My plan is to avoid changing behavior with the task store, so i did not have any intention of removing fields.
> On April 20, 2015, 5:50 p.m., Maxim Khutornenko wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, line 200
> > <https://reviews.apache.org/r/33279/diff/1/?file=932067#file932067line200>
> >
> > Isn't this too restrictive? Currently, we can still have something like 'rack':'limit:5' and 'rack':'abc'. This constraint will now allow configs like that any longer.
>
> Maxim Khutornenko wrote:
> s/now/not
FWIW you actually can't do that due to AURORA-199. I'm indifferent.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33279/#review80750
-----------------------------------------------------------
On April 16, 2015, 10 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33279/
> -----------------------------------------------------------
>
> (Updated April 16, 2015, 10 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
>
>
> Bugs: AURORA-556
> https://issues.apache.org/jira/browse/AURORA-556
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Add SQL tables needed for a datbase-backed task store.
>
>
> Diffs
> -----
>
> src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql ed73ef7a25537f145910dba7a1985805b3979173
>
> Diff: https://reviews.apache.org/r/33279/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 33279: Add SQL tables needed for a datbase-backed
task store.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33279/#review80750
-----------------------------------------------------------
src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql
<https://reviews.apache.org/r/33279/#comment130862>
What are your plans on retaining the "user" part of the Identity struct (AURORA-749)? If you are planning to address it later a follow up TODO here would be nice to avoid dropping this issue off the radar.
src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql
<https://reviews.apache.org/r/33279/#comment130882>
Isn't this too restrictive? Currently, we can still have something like 'rack':'limit:5' and 'rack':'abc'. This constraint will now allow configs like that any longer.
- Maxim Khutornenko
On April 16, 2015, 10 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33279/
> -----------------------------------------------------------
>
> (Updated April 16, 2015, 10 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
>
>
> Bugs: AURORA-556
> https://issues.apache.org/jira/browse/AURORA-556
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Add SQL tables needed for a datbase-backed task store.
>
>
> Diffs
> -----
>
> src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql ed73ef7a25537f145910dba7a1985805b3979173
>
> Diff: https://reviews.apache.org/r/33279/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 33279: Add SQL tables needed for a datbase-backed
task store.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On April 24, 2015, 6:35 p.m., Bill Farner wrote:
> > Ping - Maxim, can you check my replies above?
>
> Maxim Khutornenko wrote:
> >My plan is to avoid changing behavior with the task store, so i did not have any intention of removing fields.
>
> Sorry, I was under assumption your decided to address the "user" part. Not storing it in H2 means changing the current behavior, which I thought you wanted to avoid?
>
> Bill Farner wrote:
> Now i'm confused. The user is stored, causing behavior to be preserved.
> ```
> creator_user VARCHAR NOT NULL,
> ```
Oh, I completely missed that one! Makes sense now. Apologies for the delay.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33279/#review81512
-----------------------------------------------------------
On April 16, 2015, 10 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33279/
> -----------------------------------------------------------
>
> (Updated April 16, 2015, 10 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
>
>
> Bugs: AURORA-556
> https://issues.apache.org/jira/browse/AURORA-556
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Add SQL tables needed for a datbase-backed task store.
>
>
> Diffs
> -----
>
> src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql ed73ef7a25537f145910dba7a1985805b3979173
>
> Diff: https://reviews.apache.org/r/33279/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 33279: Add SQL tables needed for a datbase-backed
task store.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On April 24, 2015, 6:35 p.m., Bill Farner wrote:
> > Ping - Maxim, can you check my replies above?
>My plan is to avoid changing behavior with the task store, so i did not have any intention of removing fields.
Sorry, I was under assumption your decided to address the "user" part. Not storing it in H2 means changing the current behavior, which I thought you wanted to avoid?
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33279/#review81512
-----------------------------------------------------------
On April 16, 2015, 10 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33279/
> -----------------------------------------------------------
>
> (Updated April 16, 2015, 10 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
>
>
> Bugs: AURORA-556
> https://issues.apache.org/jira/browse/AURORA-556
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Add SQL tables needed for a datbase-backed task store.
>
>
> Diffs
> -----
>
> src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql ed73ef7a25537f145910dba7a1985805b3979173
>
> Diff: https://reviews.apache.org/r/33279/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 33279: Add SQL tables needed for a datbase-backed
task store.
Posted by Bill Farner <wf...@apache.org>.
> On April 24, 2015, 6:35 p.m., Bill Farner wrote:
> > Ping - Maxim, can you check my replies above?
>
> Maxim Khutornenko wrote:
> >My plan is to avoid changing behavior with the task store, so i did not have any intention of removing fields.
>
> Sorry, I was under assumption your decided to address the "user" part. Not storing it in H2 means changing the current behavior, which I thought you wanted to avoid?
Now i'm confused. The user is stored, causing behavior to be preserved.
```
creator_user VARCHAR NOT NULL,
```
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33279/#review81512
-----------------------------------------------------------
On April 16, 2015, 10 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33279/
> -----------------------------------------------------------
>
> (Updated April 16, 2015, 10 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
>
>
> Bugs: AURORA-556
> https://issues.apache.org/jira/browse/AURORA-556
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Add SQL tables needed for a datbase-backed task store.
>
>
> Diffs
> -----
>
> src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql ed73ef7a25537f145910dba7a1985805b3979173
>
> Diff: https://reviews.apache.org/r/33279/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 33279: Add SQL tables needed for a datbase-backed
task store.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33279/#review81512
-----------------------------------------------------------
Ping - Maxim, can you check my replies above?
- Bill Farner
On April 16, 2015, 10 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33279/
> -----------------------------------------------------------
>
> (Updated April 16, 2015, 10 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
>
>
> Bugs: AURORA-556
> https://issues.apache.org/jira/browse/AURORA-556
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Add SQL tables needed for a datbase-backed task store.
>
>
> Diffs
> -----
>
> src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql ed73ef7a25537f145910dba7a1985805b3979173
>
> Diff: https://reviews.apache.org/r/33279/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 33279: Add SQL tables needed for a datbase-backed
task store.
Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33279/#review80409
-----------------------------------------------------------
Ship it!
Ship It!
- Kevin Sweeney
On April 16, 2015, 3 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33279/
> -----------------------------------------------------------
>
> (Updated April 16, 2015, 3 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
>
>
> Bugs: AURORA-556
> https://issues.apache.org/jira/browse/AURORA-556
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Add SQL tables needed for a datbase-backed task store.
>
>
> Diffs
> -----
>
> src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql ed73ef7a25537f145910dba7a1985805b3979173
>
> Diff: https://reviews.apache.org/r/33279/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 33279: Add SQL tables needed for a datbase-backed
task store.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33279/#review81525
-----------------------------------------------------------
Ship it!
Ship It!
- Maxim Khutornenko
On April 16, 2015, 10 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33279/
> -----------------------------------------------------------
>
> (Updated April 16, 2015, 10 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
>
>
> Bugs: AURORA-556
> https://issues.apache.org/jira/browse/AURORA-556
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Add SQL tables needed for a datbase-backed task store.
>
>
> Diffs
> -----
>
> src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql ed73ef7a25537f145910dba7a1985805b3979173
>
> Diff: https://reviews.apache.org/r/33279/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>