aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joshua Cohen <jco...@apache.org>
Subject Re: Review Request 45112: Add support for storing and fetching images as properties of task configs.
Date Mon, 21 Mar 2016 20:48:48 GMT


> On March 21, 2016, 7:35 p.m., Maxim Khutornenko wrote:
> > api/src/main/thrift/org/apache/aurora/gen/api.thrift, line 218
> > <https://reviews.apache.org/r/45112/diff/2/?file=1308743#file1308743line218>
> >
> >     Couldn't find what it maps to in DB, is it not saved?
> 
> Joshua Cohen wrote:
>     It's done by `TaskConfigMananger` here: https://reviews.apache.org/r/45112/diff/2#2
> 
> Maxim Khutornenko wrote:
>     Ah, I guess `image_id` column name got me thinking it only supports appc. On that
note, shouldn't different image types be mapped to dedicated tables to start with? With the
move towards db-driven snapshots it may be easier to reason about future schema migrations
when there is a 1:1 denormalized mapping between thrift and DB schema.

I did that based on a discussion you and I had when I was initially investigating this. I
had originally thought to store images using the same structure as containers, and we discussed
a flattened structure there, rather than creating separate tables for each type of container.
Given the similarities between image definitions as things stand today it doesn't seem necessary
to have separate tables.

Do you feel differently now? I think this approach is reasonable given what we know about
the space, but I'm amenable to the argument that individual tables buy us more flexibility/clarity
in the future (it's really just a YAGNI question for me... do we invision an explosion of
supported image formats? At most I'd imagine one more in the foreseeable future...).


- Joshua


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45112/#review124626
-----------------------------------------------------------


On March 21, 2016, 6:20 p.m., Joshua Cohen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45112/
> -----------------------------------------------------------
> 
> (Updated March 21, 2016, 6:20 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-1635
>     https://issues.apache.org/jira/browse/AURORA-1635
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Add support for storing and fetching images as properties of task configs.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift d4b8904031e6671a8083cac9b82d934377797fe2

>   src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java b3b8ccf868c2a2f18f720a837e90d763072dd3eb

>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 364026a8ef2b47cf1beafa3990691d3375516fe6

>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 12ca16b79a062d9ea15c206ef963fb077ad7ad98

>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbImage.java PRE-CREATION

>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java eb848add00fba6d3571657bb9080be0599b2756a

>   src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml fd272ccf9b1cfccd9198d1e5e0db37d23f546afa

>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql be60c3be0cd51d06ba0ca7f56384281ee71d8c55

>   src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
1a520b3cb035a9afc25406d2f313c9f861eee4d6 
>   src/test/java/org/apache/aurora/scheduler/storage/AbstractCronJobStoreTest.java c316e497a34a45c7ada2ca83a1115e826c0f572f

>   src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java e56fed2e6c0cdb47737cf1a9b637c44c5e5b9815

>   src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java 08530397ff75081bde6f07f9d53317b5486e0da4

> 
> Diff: https://reviews.apache.org/r/45112/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew build -Pq
> 
> ran e2e tests.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message