v0.4.2 (#171)

JoshKarpel · web-flow · commit 66f95c0600fa · 2019-07-29T09:29:47.000-05:00
* resolve #168 * resolve #169 * resolve #165 * resolve #173
diff --git a/.dockerignore b/.dockerignore
@@ -108,3 +108,5 @@ venv.bak/
 
 Dockerfile
 dr
+
+!htmap-exec/singularity.d/*
diff --git a/.gitignore b/.gitignore
@@ -106,3 +106,5 @@ venv.bak/
 # Editors
 .idea/
 .vscode/
+
+!htmap-exec/singularity.d/*
diff --git a/docs/source/versions/v0_4_2.rst b/docs/source/versions/v0_4_2.rst
@@ -0,0 +1,36 @@
+v0.4.2
+======
+
+New Features
+------------
+
+Bug Fixes
+---------
+
+* `Map.errors` and `Map.error_reports()` now work when there is a mix of holds
+  and errors in the map. Previously, held components would cause both of these to
+  raise `MapComponentHeld` when trying to access them in that situation.
+  Issue: https://github.com/htcondor/htmap/issues/165
+* Requirements statement merging was broken when any of the three sources of requirements
+  (settings, function-level map options, and individual-map map options) were not given.
+  Requirements from all source are now properly merged, regardless of whether any of them
+  actually exist.
+  Issue: https://github.com/htcondor/htmap/issues/168
+* Top-level settings that were dictionaries (like ``MAP_OPTIONS``) did not behave
+  correctly when elements of them were set; they did not inherit the old settings.
+  These kinds of settings are now properly inherited, but expect breaking changes in the
+  `Settings` API next release to resolve the underlying issues.
+  Issue: https://github.com/htcondor/htmap/issues/169
+* The ``htmap-exec`` Docker image should now cleanly export to Singularity.
+  Issue: https://github.com/htcondor/htmap/issues/173
+
+Known Issues
+------------
+
+* Execution errors that result in the job being terminated but no output being
+  produced are still not handled entirely gracefully. Right now, the component
+  state will just show as ``ERRORED``, but there won't be an actual error report.
+* Map component state may become corrupted when a map is manually vacated.
+  Force-removal may be needed to clean up maps if HTCondor and HTMap disagree
+  about the state of their components.
+  Issue: https://github.com/htcondor/htmap/issues/129
diff --git a/htmap-exec/Dockerfile b/htmap-exec/Dockerfile
@@ -13,14 +13,16 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-FROM continuumio/anaconda3:latest
+FROM continuumio/anaconda3:2019.03
 
 LABEL maintainer="karpel@wisc.edu"
 
 COPY . /tmp/htmap
-RUN pip install --no-cache-dir /tmp/htmap/ \
+RUN python3 -m pip install --no-cache-dir /tmp/htmap/ \
  && rm -rf /tmp/htmap
 
+COPY htmap-exec/singularity.d /.singularity.d
+
 ARG USER=htmap
 RUN groupadd ${USER} \
  && useradd -m -g ${USER} ${USER}
diff --git a/htmap-exec/singularity.d/env/90-environment.sh b/htmap-exec/singularity.d/env/90-environment.sh
@@ -0,0 +1 @@
+export PATH=/opt/conda/bin:$PATH
diff --git a/htmap/maps.py b/htmap/maps.py
@@ -672,7 +672,9 @@ def errors(self) -> Dict[int, errors.ComponentError]:
         for idx in self.components:
             try:
                 err[idx] = self.get_err(idx)
-            except (exceptions.OutputNotFound, exceptions.ExpectedError) as e:
+            except (exceptions.OutputNotFound,
+                    exceptions.ExpectedError,
+                    exceptions.MapComponentHeld) as e:
                 pass
 
         return err
@@ -684,7 +686,10 @@ def error_reports(self) -> Iterator[str]:
         for idx in self.components:
             try:
                 yield self.get_err(idx, timeout = 0).report()
-            except (exceptions.OutputNotFound, exceptions.ExpectedError, exceptions.TimeoutError) as e:
+            except (exceptions.OutputNotFound,
+                    exceptions.ExpectedError,
+                    exceptions.TimeoutError,
+                    exceptions.MapComponentHeld) as e:
                 pass
 
     @property
diff --git a/htmap/options.py b/htmap/options.py
@@ -173,11 +173,10 @@ def create_submit_object_and_itemdata(
         settings['DELIVERY_METHOD'],
     )
 
-    # todo: needs test
-    base_requirements = descriptors.get('requirements', None)
-    extra_requirements = map_options.pop('requirements', None)
-    if base_requirements is not None and extra_requirements is not None:
-        descriptors['requirements'] = f'({base_requirements}) && ({extra_requirements})'
+    descriptors['requirements'] = merge_requirements(
+        descriptors.get('requirements', None),
+        map_options.get('requirements', None),
+    )
 
     itemdata = [{'component': str(idx)} for idx in range(num_components)]
 
@@ -214,6 +213,9 @@ def create_submit_object_and_itemdata(
         else:
             descriptors[opt_key] = opt_value
 
+    if descriptors['requirements'] is None:
+        descriptors.pop('requirements')
+
     sub = htcondor.Submit(descriptors)
 
     return sub, itemdata
@@ -236,6 +238,13 @@ def unregister_delivery_mechanism(name: str) -> None:
     SETUP_FUNCTION_BY_DELIVERY.pop(name)
 
 
+def merge_requirements(*requirements: Optional[str]) -> Optional[str]:
+    requirements = [req for req in requirements if req is not None]
+    if len(requirements) == 0:
+        return None
+    return ' && '.join(f'({req})' for req in requirements)
+
+
 def get_base_descriptors(
     tag: str,
     map_dir: Path,
@@ -265,17 +274,21 @@ def get_base_descriptors(
 
     from_settings = settings.get('MAP_OPTIONS', default = {})
 
-    base_requirements = base.pop('requirements', None)
-    settings_requirements = from_settings.pop('requirements', None)
-    if base_requirements is not None and settings_requirements is not None:
-        core['requirements'] = f'({base_requirements}) && ({settings_requirements})'
-
-    return {
+    merged = {
         **core,
         **base,
         **from_settings,
     }
 
+    # manually fix-up requirements
+    merged['requirements'] = merge_requirements(
+        core.get('requirements', None),
+        base.get('requirements', None),
+        from_settings.get('requirements', None),
+    )
+
+    return merged
+
 
 def _copy_run_scripts():
     run_script_source_dir = Path(__file__).parent / names.RUN_DIR
diff --git a/htmap/settings.py b/htmap/settings.py
@@ -49,17 +49,14 @@ def __init__(self, *settings):
         self.maps = list(settings)
 
     def __getitem__(self, key: str):
-        for map in self.maps:
+        try:
             path = key.split('.')
-            r = map
-            try:
-                for component in path:
-                    r = r[component]
-            except (KeyError, TypeError):
-                continue
+            r = self.to_dict()
+            for component in path:
+                r = r[component]
             return r
-
-        raise exceptions.MissingSetting()
+        except (KeyError, TypeError):
+            raise exceptions.MissingSetting()
 
     def __eq__(self, other: Any) -> bool:
         return type(self) is type(other) and self.to_dict() == other.to_dict()
@@ -145,7 +142,7 @@ def __str__(self) -> str:
         return utils.rstr(toml.dumps(self.to_dict()))
 
     def __repr__(self) -> str:
-        return utils.rstr(f'<{self.__class__.__name__}>')
+        return f'<{self.__class__.__name__}>'
 
 
 htmap_dir = Path(os.getenv('HTMAP_DIR', Path.home() / '.htmap'))
diff --git a/htmap/version.py b/htmap/version.py
@@ -15,7 +15,7 @@
 
 from typing import Tuple
 
-__version__ = '0.4.1'
+__version__ = '0.4.2'
 
 
 def version() -> str:
diff --git a/tests/_inf/.htmaprc b/tests/_inf/.htmaprc
@@ -1,4 +1,4 @@
 DELIVERY_METHOD = "assume"
 
 [MAP_OPTIONS]
-REQUEST_DISK = "100MB"
+request_disk = "100MB"
diff --git a/tests/integration/test_errors_and_holds.py b/tests/integration/test_errors_and_holds.py
@@ -0,0 +1,76 @@
+# Copyright 2019 HTCondor Team, Computer Sciences Department,
+# University of Wisconsin-Madison, WI.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from pathlib import Path
+
+import pytest
+
+import htmap
+
+import htcondor
+
+
+@pytest.fixture(scope = 'function')
+def hold_before_error():
+    map = htmap.map(lambda x: 1 / x, [1, 0])
+
+    schedd = htcondor.Schedd()
+    cluster_id = map._cluster_ids[0]
+    schedd.act(htcondor.JobAction.Hold, f"(ClusterID == {cluster_id}) && (ProcID == 0)")
+
+    map.wait(holds_ok = True, errors_ok = True)
+
+    assert map.component_statuses == [htmap.ComponentStatus.HELD, htmap.ComponentStatus.ERRORED]
+
+    return map
+
+
+@pytest.fixture(scope = 'function')
+def error_before_hold():
+    map = htmap.map(lambda x: 1 / x, [0, 1])
+
+    schedd = htcondor.Schedd()
+    cluster_id = map._cluster_ids[0]
+    schedd.act(htcondor.JobAction.Hold, f"(ClusterID == {cluster_id}) && (ProcID == 1)")
+
+    map.wait(holds_ok = True, errors_ok = True)
+
+    assert map.component_statuses == [htmap.ComponentStatus.ERRORED, htmap.ComponentStatus.HELD]
+
+    return map
+
+
+def test_can_get_error_if_hold_in_front(hold_before_error):
+    hold_before_error.get_err(1)
+
+
+def test_can_get_error_if_error_in_front(error_before_hold):
+    error_before_hold.get_err(0)
+
+
+def test_can_iterate_over_errors_if_hold_in_front(hold_before_error):
+    assert len(list(hold_before_error.error_reports())) == 1
+
+
+def test_can_iterate_over_errors_if_error_in_front(error_before_hold):
+    assert len(list(error_before_hold.error_reports())) == 1
+
+
+def test_can_get_errors_if_hold_in_front(hold_before_error):
+    assert len(list(hold_before_error.errors)) == 1
+
+
+def test_can_get_errors_if_error_in_front(error_before_hold):
+    assert len(list(error_before_hold.errors)) == 1
diff --git a/tests/unit/test_requirements.py b/tests/unit/test_requirements.py
@@ -0,0 +1,60 @@
+# Copyright 2018 HTCondor Team, Computer Sciences Department,
+# University of Wisconsin-Madison, WI.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import pytest
+
+from htmap.options import merge_requirements
+
+
+def test_merge_nothing():
+    req = merge_requirements()
+
+    assert req is None
+
+
+def test_merge_one():
+    req = merge_requirements(
+        'HasPie == true',
+    )
+
+    assert req == '(HasPie == true)'
+
+
+def test_merge_two():
+    req = merge_requirements(
+        'HasPie == true',
+        'HasCake == false',
+    )
+
+    assert req == '(HasPie == true) && (HasCake == false)'
+
+
+def test_merge_three():
+    req = merge_requirements(
+        'HasPie == true',
+        'HasCake == false',
+        'IsHappy == true',
+    )
+
+    assert req == '(HasPie == true) && (HasCake == false) && (IsHappy == true)'
+
+
+def test_merge_with_none():
+    req = merge_requirements(
+        'HasPie == true',
+        None,
+    )
+
+    assert req == '(HasPie == true)'
diff --git a/tests/unit/test_settings.py b/tests/unit/test_settings.py
@@ -216,3 +216,11 @@ def test_can_load_saved_settings(tmpdir):
 
     assert loaded == s
     assert loaded is not s
+
+
+def test_nested_dicts_are_not_hidden_by_set():
+    s = Settings({'top': {'hidden': 0}})
+
+    s['top.new'] = 5
+
+    assert s['top'] == {'hidden': 0, 'new': 5}
