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:
[email protected]


With regards,
Apache Git Services

Reply via email to