[GitHub] [fluo-muchos] sputnik13 commented on a change in pull request #288: Refactor config

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

sputnik13 commented on a change in pull request #288: Refactor config
URL: https://github.com/apache/fluo-muchos/pull/288#discussion_r332232724
 
 

 ##########
 File path: lib/muchos/config/base.py
 ##########
 @@ -117,73 +202,32 @@ def mounts(self, num_ephemeral):
             mounts.append(self.mount_root() + str(i))
         return mounts
 
+    @abstractmethod
+    @ansible_play_var
     def mount_root(self):
-        if self.get_cluster_type() == 'ec2':
-            return '/media/' + self.ephemeral_root
-        elif self.get_cluster_type() == 'existing':
-            return self.get('existing', 'mount_root')
-        elif self.get_cluster_type() == 'azure':
-            return self.get('azure', 'mount_root')
-
-    def fstype(self):
-        retval = None
-        if self.get_cluster_type() == 'ec2':
-            retval = self.get('ec2', 'fstype')
-            if not retval:
-                return 'ext3'
-        return retval
-
-    def force_format(self):
-        retval = 'no'
-        if self.get_cluster_type() == 'ec2':
-            retval = self.get('ec2', 'force_format')
-            if not retval:
-                return 'no'
-        return retval
+        raise NotImplementedError()
 
+    @abstractmethod
     def data_dirs_common(self, nodeType):
-        data_dirs = []
-
-        if self.get_cluster_type() == 'ec2':
-            data_dirs = self.node_type_map()[nodeType]['mounts']
-        elif self.get_cluster_type() == 'existing':
-            data_dirs = self.get('existing', 'data_dirs').split(",")
-        elif self.get_cluster_type() == 'azure':
-            num_disks = int(self.get("azure", "numdisks"))
-            range_var = num_disks + 1
-            for diskNum in range(1, range_var):
-                data_dirs.append(self.get("azure", "mount_root") +
-                                 str(diskNum))
-
-        return data_dirs
+        raise NotImplementedError()
 
+    @ansible_host_var
     def worker_data_dirs(self):
         return self.data_dirs_common("worker")
 
+    @ansible_host_var
     def default_data_dirs(self):
         return self.data_dirs_common("default")
 
+    @abstractmethod
+    @ansible_play_var
     def metrics_drive_ids(self):
-        if self.get_cluster_type() == 'ec2':
-            drive_ids = []
-            for i in range(0, self.max_ephemeral()):
-                drive_ids.append(self.metrics_drive_root + str(i))
-            return drive_ids
-        elif self.get_cluster_type() == 'existing':
-            return self.get("existing", "metrics_drive_ids").split(",")
-        elif self.get_cluster_type() == 'azure':
-            drive_ids = []
-            range_var = int(self.get("azure", "numdisks")) + 1
-            for i in range(1, range_var):
-                drive_ids.append(self.get("azure", "metrics_drive_root") +
-                                 str(i))
-            return drive_ids
+        raise NotImplementedError()
 
+    @abstractmethod
+    @ansible_play_var
     def shutdown_delay_minutes(self):
 
 Review comment:
   I think that's probably fine as well, I didn't have sufficient context to be able to say what makes more sense to have a default/base implementation vs force to be reimplemented, open to feedback on this point since you have much more insight as to how these variables are used.  I've only worked with and tested the azure installations.

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