[GitHub] [aurora] rdelval commented on a change in pull request #59: Feature add docker/volume and volume/secret support (#58)

[GitBox <gi...@apache.org>]

rdelval commented on a change in pull request #59:     Feature add docker/volume and volume/secret support (#58)
URL: https://github.com/apache/aurora/pull/59#discussion_r297458003
 
 

 ##########
 File path: src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 ##########
 @@ -476,6 +478,18 @@ public ITaskConfig validateAndPopulate(
       if (!settings.allowContainerVolumes && !container.getVolumes().isEmpty()) {
         throw new TaskDescriptionException(NO_CONTAINER_VOLUMES);
       }
+
+      if (settings.allowContainerVolumes  && !container.getVolumes().isEmpty()) {
+        builder.setContainer(Container.mesos(
+            container.newBuilder()
+                .setVolumes(container.getVolumes().stream()
+                    .map(v -> v.isSetVolumeType() ? v.newBuilder() : v.newBuilder()
 
 Review comment:
   Since in the api.thrift we set a default value on line 223 https://github.com/apache/aurora/pull/59/files#diff-03d58f7b857e5429e1668264857798e1R223 do we ever hit the case where the volume is not set? Or does thrift not assign anything if we don't explicitly set anything? Just wondering because having a value be optional and have a default value at the same time is counter intuitive.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services