diff --git a/spark8t/domain.py b/spark8t/domain.py index cc4f5b7..8efef7a 100644 --- a/spark8t/domain.py +++ b/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 diff --git a/spark8t/services.py b/spark8t/services.py index 044554c..07145a5 100644 --- a/spark8t/services.py +++ b/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( diff --git a/tests/unittest/test_domain.py b/tests/unittest/test_domain.py index f248ee4..efd9451 100644 --- a/tests/unittest/test_domain.py +++ b/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))