Address review comments

perfmetrics/scripts/testing_on_gke/examples/dlio/parse_logs.py

@@ -15,13 +15,17 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+# standard library imports
 import argparse
-import json, os, pprint, subprocess
-from os.path import dirname
+import json
+import os
+import pprint
+import subprocess
 import sys
-import dlio_workload
 
+# local library imports
 sys.path.append("../")
+import dlio_workload
 from utils.utils import get_memory, get_cpu, standard_timestamp, is_mash_installed
 
 _LOCAL_LOGS_LOCATION = "../../bin/dlio-logs/logs"

perfmetrics/scripts/testing_on_gke/examples/dlio/run_tests.py

@@ -27,6 +27,10 @@
 import dlio_workload
 
 
+# The default value of gcsfuse-mount-options to be used
+# for "gcsfuse-generic" scenario.
+# For description of how to specify the value for this,
+# look at the description of the argparser argument for gcsfuse-mount-options.
 _DEFAULT_GCSFUSE_MOUNT_OPTIONS = 'implicit-dirs'
 
 
@@ -47,8 +51,8 @@ def createHelmInstallCommands(
     instanceId: str,
     gcsfuseMountOptions: str,
     machineType: str,
-):
-  """Create helm install commands for the given dlioWorkload objects."""
+) -> list:
+  """Creates helm install commands for the given dlioWorkload objects."""
   helm_commands = []
   if not gcsfuseMountOptions:
     gcsfuseMountOptions = _DEFAULT_GCSFUSE_MOUNT_OPTIONS
@@ -105,7 +109,7 @@ def main(args) -> None:
   parser.add_argument(
       '--workload-config',
       metavar='JSON workload configuration file path',
-      help='Runs DLIO Unet3d tests using this JSON workload configuration.',
+      help='Runs DLIO Unet3d tests from this JSON workload configuration file.',
       required=True,
   )
   parser.add_argument(
@@ -121,8 +125,14 @@ def main(args) -> None:
       '--gcsfuse-mount-options',
       metavar='GCSFuse mount options',
       help=(
-          'GCSFuse mount-options, in JSON stringified format, to be set for the'
-          ' scenario gcsfuse-generic.'
+          'GCSFuse mount-options, in a compact stringified'
+          ' format, to be set for the '
+          ' scenario "gcsfuse-generic". The individual config/cli flag values'
+          ' should be separated by comma. Each cli flag should be of the form'
+          ' "<name>[=<value>]". Each config-file flag should be of form'
+          ' "<config>[:<subconfig>[:<subsubconfig>[...]]]:<value>". For'
+          ' example, a sample value would be:'
+          ' "implicit-dirs,file_mode=777,file-cache:enable-parallel-downloads:true,metadata-cache:ttl-secs:-1".'
       ),
       required=False,
   )
@@ -143,9 +153,19 @@ def main(args) -> None:
   )
 
   args = parser.parse_args()
-  if ' ' in args.instance_id:
-    raise Exception('Argument --instance-id contains space in it')
-  if len(args.machine_type) == 0 or str.isspace(args.machine_type):
-    raise Exception('Argument --machine-type is empty or only spaces')
+  for argument in ['instance_id', 'gcsfuse_mount_options', 'machine_type']:
+    value = getattr(args, argument)
+    if ' ' in value:
+      raise Exception(
+          f'Argument {argument} (value="{value}") contains space in it, which'
+          ' is not supported.'
+      )
+  for argument in ['machine_type', 'instance_id']:
+    value = getattr(args, argument)
+    if len(value) == 0 or str.isspace(value):
+      raise Exception(
+          f'Argument {argument} (value="{value}") is empty or contains only'
+          ' spaces.'
+      )
 
   main(args)

perfmetrics/scripts/testing_on_gke/examples/fio/parse_logs.py

@@ -15,14 +15,18 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+# standard library imports
 import argparse
-import json, os, pprint, subprocess
-from os.path import dirname
+import json
+import os
+import pprint
+import subprocess
 import sys
-import fio_workload
 
+# local library imports
 sys.path.append("../")
-from utils.utils import get_memory, get_cpu, unix_to_timestamp, is_mash_installed
+import fio_workload
+from utils.utils import get_memory, get_cpu, standard_timestamp, is_mash_installed
 
 _LOCAL_LOGS_LOCATION = "../../bin/fio-logs"
 

perfmetrics/scripts/testing_on_gke/examples/fio/run_tests.py

@@ -26,6 +26,10 @@
 import fio_workload
 
 
+# The default value of gcsfuse-mount-options to be used
+# for "gcsfuse-generic" scenario.
+# For description of how to specify the value for this,
+# look at the description of the argparser argument for gcsfuse-mount-options.
 _DEFAULT_GCSFUSE_MOUNT_OPTIONS = 'implicit-dirs'
 
 
@@ -106,7 +110,7 @@ def main(args) -> None:
   parser.add_argument(
       '--workload-config',
       metavar='JSON workload configuration file path',
-      help='Runs FIO tests using this JSON workload configuration',
+      help='Runs FIO tests from this JSON workload configuration file.',
       required=True,
   )
   parser.add_argument(
@@ -122,8 +126,14 @@ def main(args) -> None:
       '--gcsfuse-mount-options',
       metavar='GCSFuse mount options',
       help=(
-          'GCSFuse mount-options, in JSON stringified format, to be set for the'
-          ' scenario gcsfuse-generic.'
+          'GCSFuse mount-options, in a compact stringified'
+          ' format, to be set for the '
+          ' scenario "gcsfuse-generic". The individual config/cli flag values'
+          ' should be separated by comma. Each cli flag should be of the form'
+          ' "<name>[=<value>]". Each config-file flag should be of form'
+          ' "<config>[:<subconfig>[:<subsubconfig>[...]]]:<value>". For'
+          ' example, a sample value would be:'
+          ' "implicit-dirs,file_mode=777,file-cache:enable-parallel-downloads:true,metadata-cache:ttl-secs:-1".'
       ),
       required=False,
   )
@@ -144,9 +154,19 @@ def main(args) -> None:
   )
 
   args = parser.parse_args()
-  if ' ' in args.instance_id:
-    raise Exception('Argument --instance-id contains space in it')
-  if len(args.machine_type) == 0 or str.isspace(args.machine_type):
-    raise Exception('Argument --machine-type is empty or only spaces')
+  for argument in ['instance_id', 'gcsfuse_mount_options', 'machine_type']:
+    value = getattr(args, argument)
+    if ' ' in value:
+      raise Exception(
+          f'Argument {argument} (value="{value}") contains space in it, which'
+          ' is not supported.'
+      )
+  for argument in ['machine_type', 'instance_id']:
+    value = getattr(args, argument)
+    if len(value) == 0 or str.isspace(value):
+      raise Exception(
+          f'Argument {argument} (value="{value}") is empty or contains only'
+          ' spaces.'
+      )
 
   main(args)