> 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 > >
