[DPE-2272] Fix parsing of empty lines in config files (#31)

This PR resolves #26.

spark8t/domain.py

@@ -32,6 +32,21 @@ def _is_property_with_options(key: str) -> bool:
         """
         return key in ["spark.driver.extraJavaOptions"]
 
+    @staticmethod
+    def is_line_parsable(line: str) -> bool:
+        """Check if a given line is parsable(not empty or commented).
+
+        Args:
+            line: a line of the configuration
+        """
+        # empty line
+        if len(line.strip()) == 0:
+            return False
+        # commented line
+        elif line.strip().startswith("#"):
+            return False
+        return True
+
     @staticmethod
     def parse_property_line(line: str) -> Tuple[str, str]:
         prop_assignment = list(filter(None, re.split("=| ", line.strip())))
@@ -53,6 +68,9 @@ def _read_property_file_unsafe(cls, name: str) -> Dict:
         defaults = dict()
         with open(name) as f:
             for line in f:
+                # skip empty or commented line
+                if not PropertyFile.is_line_parsable(line):
+                    continue
                 key, value = cls.parse_property_line(line)
                 defaults[key] = os.path.expandvars(value)
         return defaults

spark8t/services.py

@@ -359,6 +359,8 @@ def get_service_accounts(
         labels_to_pass = dict()
         if labels:
             for entry in labels:
+                if not PropertyFile.is_line_parsable(entry):
+                    continue
                 k, v = PropertyFile.parse_property_line(entry)
                 labels_to_pass[k] = v
 
@@ -1459,7 +1461,11 @@ def _generate_properties_file_from_arguments(confs: List[str]):
             return PropertyFile({})
 
         return PropertyFile(
-            dict(PropertyFile.parse_property_line(line) for line in confs)
+            dict(
+                PropertyFile.parse_property_line(line)
+                for line in confs
+                if PropertyFile.is_line_parsable(line)
+            )
         )
 
     def spark_submit(

tests/unittest/test_domain.py

@@ -1,4 +1,5 @@
 import logging
+import tempfile
 import uuid
 
 from spark8t.domain import Defaults, PropertyFile, ServiceAccount
@@ -72,36 +73,86 @@ def test_service_account():
     assert sa.configurations.props.get("spark.dummy.property1") == spark_dummy_property1
     assert sa.configurations.props.get("spark.dummy.property2") == spark_dummy_property2
 
-    def test_property_removing_conf(self):
-        confs = ["key1=value1", "key2=value2", "key3=value3"]
 
-        prop = PropertyFile(
-            dict(PropertyFile.parse_property_line(line) for line in confs)
-        )
+def test_property_removing_conf():
+    """
+    Validates removal of configuration options.
+    """
+    confs = ["key1=value1", "key2=value2", "key3=value3"]
 
-        self.assertFalse("key1" in prop.remove(["key1"]).props)
+    prop = PropertyFile(dict(PropertyFile.parse_property_line(line) for line in confs))
 
-        self.assertTrue("key3" in prop.remove(["key1", "key2"]).props)
+    assert "key1" not in prop.remove(["key1"]).props
 
-        self.assertDictEqual(prop.props, prop.remove([]).props)
+    assert "key3" in prop.remove(["key1", "key2"]).props
 
-    def test_property_removing_conf_with_pairs(self):
-        confs = ["key1=value1", "key2=value2", "key3=value3"]
+    assert prop.props == prop.remove([]).props
 
-        prop = PropertyFile(
-            dict(PropertyFile.parse_property_line(line) for line in confs)
-        )
 
-        self.assertFalse("key1" in prop.remove(["key1=value1"]).props)
+def test_property_removing_conf_with_pairs():
+    """
+    Validates the correct removal of property pairs.
+    """
+    confs = ["key1=value1", "key2=value2", "key3=value3"]
+
+    prop = PropertyFile(dict(PropertyFile.parse_property_line(line) for line in confs))
+
+    assert "key1" not in prop.remove(["key1=value1"]).props
+
+    assert "key1" in prop.remove(["key1=value2"]).props
 
-        self.assertTrue("key1" in prop.remove(["key1=value2"]).props)
+    assert "key1" not in prop.remove(["key1=value2", "key1=value1"]).props
 
-        self.assertFalse("key1" in prop.remove(["key1=value2", "key1=value1"]).props)
+    assert "key1" not in prop.remove(["key1", "key1=value2"]).props
 
-        self.assertFalse("key1" in prop.remove(["key1", "key1=value2"]).props)
+
+def test_property_empty_lines():
+    """
+    Validates that empty lines are skipped and configuration is parsed correctly.
+    """
+    confs = [
+        "key1=value1",
+        "",
+        "key2=value2",
+        "key3=value3",
+        "",
+        "#key4=value4",
+        " #key5=value5",
+    ]
+
+    with tempfile.NamedTemporaryFile(mode="w+t") as f:
+        # write conf file
+        for conf in confs:
+            f.write(f"{conf}\n")
+        f.flush()
+
+        with open(f.name, "r") as fp:
+            assert len(fp.readlines()) == 7
+
+        # read property file from temporary file name
+        prop = PropertyFile.read(f.name)
+
+        assert "key1" not in prop.remove(["key1=value1"]).props
+
+        assert "key1" in prop.remove(["key1=value2"]).props
+
+        assert "key1" not in prop.remove(["key1=value2", "key1=value1"]).props
+
+        assert "key1" not in prop.remove(["key1", "key1=value2"]).props
+
+        assert "key4" not in prop.props
+
+        assert "#key4" not in prop.props
+
+        assert "key5" not in prop.props
+
+        assert "#key5" not in prop.props
 
 
 def test_property_file_parsing_from_confs():
+    """
+    Validates parsing of configuration from list.
+    """
     confs = ["key1=value1", "key2=value2"]
 
     prop = PropertyFile(dict(PropertyFile.parse_property_line(line) for line in confs))