Repository: MatterMiners/tardis

Issue:
Sleep duration of Tardis drones should be configurable
Hi there!

We would appreciate if the response time of the Tardis drones could be configurable (see https://github.com/MatterMiners/tardis/blob/591987aa4fa241280c3254873eed7ec8aad2d4a1/tardis/resources/drone.py#L101), so that we can enforce a faster response time of the system if needed, e.g. in the interactive analysis use-case.

Cheers
Stefan

Discussion:
codecov-io: # [Codecov](https://codecov.io/gh/MatterMiners/tardis/pull/166?src=pr&el=h1) Report
> Merging [#166](https://codecov.io/gh/MatterMiners/tardis/pull/166?src=pr&el=desc) (e4e3506) into [master](https://codecov.io/gh/MatterMiners/tardis/commit/591987aa4fa241280c3254873eed7ec8aad2d4a1?el=desc) (591987a) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/MatterMiners/tardis/pull/166/graphs/tree.svg?width=650&height=150&src=pr&token=BKeuDMxoaj)](https://codecov.io/gh/MatterMiners/tardis/pull/166?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master     #166   +/-   ##
=======================================
  Coverage   99.52%   99.52%           
=======================================
  Files          42       42           
  Lines        1682     1694   +12     
=======================================
+ Hits         1674     1686   +12     
  Misses          8        8           
```


| [Impacted Files](https://codecov.io/gh/MatterMiners/tardis/pull/166?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [tardis/agents/siteagent.py](https://codecov.io/gh/MatterMiners/tardis/pull/166/diff?src=pr&el=tree#diff-dGFyZGlzL2FnZW50cy9zaXRlYWdlbnQucHk=) | `100.00% <100.00%> (ø)` | |
| [tardis/interfaces/siteadapter.py](https://codecov.io/gh/MatterMiners/tardis/pull/166/diff?src=pr&el=tree#diff-dGFyZGlzL2ludGVyZmFjZXMvc2l0ZWFkYXB0ZXIucHk=) | `100.00% <100.00%> (ø)` | |
| [tardis/resources/drone.py](https://codecov.io/gh/MatterMiners/tardis/pull/166/diff?src=pr&el=tree#diff-dGFyZGlzL3Jlc291cmNlcy9kcm9uZS5weQ==) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/MatterMiners/tardis/pull/166?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/MatterMiners/tardis/pull/166?src=pr&el=footer). Last update [591987a...e4e3506](https://codecov.io/gh/MatterMiners/tardis/pull/166?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/docs/source/adapters/site.rst b/docs/source/adapters/site.rst
index 891273d..98a133b 100644
--- a/docs/source/adapters/site.rst
+++ b/docs/source/adapters/site.rst
@@ -31,17 +31,19 @@ Available configuration options
 
 .. container:: left-col
 
-    +------------------------+-----------------------------------------------------------------------------------------------------------------------+---------------+
-    | Option                 | Short Description                                                                                                     |  Requirement  |
-    +========================+=======================================================================================================================+===============+
-    | name                   | Name of the site                                                                                                      |  **Required** |
-    +------------------------+-----------------------------------------------------------------------------------------------------------------------+---------------+
-    | adapter                | Site adapter to use. Adapter will be auto-imported (class name without Adapter)                                       |  **Required** |
-    +------------------------+-----------------------------------------------------------------------------------------------------------------------+---------------+
-    | quota                  | Core quota to be used for this site. Negative values are interpreted as infinity                                      |  **Required** |
-    +------------------------+-----------------------------------------------------------------------------------------------------------------------+---------------+
-    | drone_minimum_lifetime | Time in seconds the drone will remain in :py:class:`~tardis.resources.dronestates.AvailableState` before draining it. |  **Optional** |
-    +------------------------+-----------------------------------------------------------------------------------------------------------------------+---------------+
+    +--------------------------+-----------------------------------------------------------------------------------------------------------------------+---------------+
+    | Option                   | Short Description                                                                                                     |  Requirement  |
+    +==========================+=======================================================================================================================+===============+
+    | name                     | Name of the site                                                                                                      |  **Required** |
+    +--------------------------+-----------------------------------------------------------------------------------------------------------------------+---------------+
+    | adapter                  | Site adapter to use. Adapter will be auto-imported (class name without Adapter)                                       |  **Required** |
+    +--------------------------+-----------------------------------------------------------------------------------------------------------------------+---------------+
+    | quota                    | Core quota to be used for this site. Negative values are interpreted as infinity                                      |  **Required** |
+    +--------------------------+-----------------------------------------------------------------------------------------------------------------------+---------------+
+    | drone_heartbeat_interval | Time in seconds between two consecutive executions of :py:meth:`tardis.resources.drone.run`. Defaults to 60s.         |  **Optional** |
+    +--------------------------+-----------------------------------------------------------------------------------------------------------------------+---------------+
+    | drone_minimum_lifetime   | Time in seconds the drone will remain in :py:class:`~tardis.resources.dronestates.AvailableState` before draining it. |  **Optional** |
+    +--------------------------+-----------------------------------------------------------------------------------------------------------------------+---------------+
 
     For each site in the `Sites` configuration block. A site specific configuration block carrying the site name
     has to be added to the configuration as well.
@@ -67,6 +69,7 @@ Available configuration options
           - name: MySiteName_1
             adapter: MyAdapter2Use
             quota: 123
+            drone_heartbeat_interval: 10
             drone_minimum_lifetime: 3600
           - name: MySiteName_2
             adapter: OtherAdapter2Use
diff --git a/docs/source/changelog.rst b/docs/source/changelog.rst
index 48aa4d4..c2c4f06 100644
--- a/docs/source/changelog.rst
+++ b/docs/source/changelog.rst
@@ -1,4 +1,4 @@
-.. Created by changelog.py at 2021-03-18, command
+.. Created by changelog.py at 2021-03-22, command
    '/Users/giffler/.cache/pre-commit/repor6pnmwlm/py_env-python3.9/bin/changelog docs/source/changes compile --output=docs/source/changelog.rst'
    based on the format of 'https://keepachangelog.com/'
 
@@ -6,9 +6,14 @@
 CHANGELOG
 #########
 
-[Unreleased] - 2021-03-18
+[Unreleased] - 2021-03-22
 =========================
 
+Added
+-----
+
+* An optional and per site configurable drone heartbeat interval has been added
+
 Fixed
 -----
 
diff --git a/docs/source/changes/166.add_drone_heartbeat_interval.yaml b/docs/source/changes/166.add_drone_heartbeat_interval.yaml
new file mode 100644
index 0000000..eabeaa4
--- /dev/null
+++ b/docs/source/changes/166.add_drone_heartbeat_interval.yaml
@@ -0,0 +1,9 @@
+category: added
+summary: "An optional and per site configurable drone heartbeat interval has been added"
+description: |
+  Add an optional and per site configurable drone heartbeat interval to TARDIS. The heartbeat interval is defined as
+  the time between two consecutive calls of the drones run method. The heartbeat interval defaults to 60s.
+issues:
+- 165
+pull requests:
+- 166
diff --git a/tardis/agents/siteagent.py b/tardis/agents/siteagent.py
index ced8978..09ff606 100644
--- a/tardis/agents/siteagent.py
+++ b/tardis/agents/siteagent.py
@@ -31,6 +31,10 @@ class SiteAgent(SiteAdapter):
     ):
         return NotImplemented
 
+    @property
+    def drone_heartbeat_interval(self) -> int:
+        return self._site_adapter.drone_heartbeat_interval
+
     @property
     def drone_minimum_lifetime(self) -> int:
         return self._site_adapter.drone_minimum_lifetime
diff --git a/tardis/interfaces/siteadapter.py b/tardis/interfaces/siteadapter.py
index 390140a..42e91c8 100644
--- a/tardis/interfaces/siteadapter.py
+++ b/tardis/interfaces/siteadapter.py
@@ -22,6 +22,7 @@ class SiteConfigurationModel(BaseModel):
     adapter: str
     quota: Optional[int] = inf
     drone_minimum_lifetime: Optional[conint(gt=0)] = None
+    drone_heartbeat_interval: Optional[conint(ge=0)] = 60
 
     class Config:
         extra = "forbid"
@@ -104,6 +105,16 @@ class SiteAdapter(metaclass=ABCMeta):
 
         return drone_environment
 
+    @property
+    def drone_heartbeat_interval(self) -> int:
+        """
+        Property that returns the configuration parameter drone_heartbeat_interval.
+        It describes the time between two consecutive updates of the drone status.
+        :return: The heartbeat interval of the drone
+        :rtype: int
+        """
+        return self.site_configuration.drone_heartbeat_interval
+
     @property
     def drone_minimum_lifetime(self) -> [int, None]:
         """
diff --git a/tardis/resources/drone.py b/tardis/resources/drone.py
index 252cc49..2cb7bbf 100644
--- a/tardis/resources/drone.py
+++ b/tardis/resources/drone.py
@@ -69,7 +69,11 @@ class Drone(Pool):
         self._demand = value
 
     @property
-    def drone_minimum_lifetime(self) -> [int, None]:
+    def heartbeat_interval(self) -> int:
+        return self.site_agent.drone_heartbeat_interval
+
+    @property
+    def minimum_lifetime(self) -> [int, None]:
         return self.site_agent.drone_minimum_lifetime
 
     @property
@@ -98,7 +102,7 @@ class Drone(Pool):
                 )
                 self._demand = 0
                 return
-            await asyncio.sleep(60)
+            await asyncio.sleep(self.heartbeat_interval)
 
     def register_plugins(self, observer: Union[List[Plugin], Plugin]) -> None:
         self._plugins.append(observer)
diff --git a/tardis/resources/dronestates.py b/tardis/resources/dronestates.py
index 087c22d..6871ef9 100644
--- a/tardis/resources/dronestates.py
+++ b/tardis/resources/dronestates.py
@@ -45,9 +45,9 @@ async def check_minimum_lifetime(
     state_transition, drone: "Drone", current_state: Type[State]
 ):
     if (
-        drone.drone_minimum_lifetime
+        drone.minimum_lifetime
         and (datetime.now() - drone.resource_attributes.updated).total_seconds()
-        > drone.drone_minimum_lifetime
+        > drone.minimum_lifetime
     ):
         raise StopProcessing(last_result=DrainState())
     return state_transition


Test patch:
diff --git a/tests/agents_t/test_siteagent.py b/tests/agents_t/test_siteagent.py
index 4640b3f..54dd80b 100644
--- a/tests/agents_t/test_siteagent.py
+++ b/tests/agents_t/test_siteagent.py
@@ -23,6 +23,11 @@ class TestSiteAgent(TestCase):
         self.site_agent.drone_uuid(uuid="test")
         self.site_adapter.drone_uuid.assert_called_with(uuid="test")
 
+    def test_drone_heartbeat_interval(self):
+        self.site_adapter.drone_heartbeat_interval.return_value = 60
+        self.assertEqual(self.site_agent.drone_heartbeat_interval(), 60)
+        self.site_adapter.drone_heartbeat_interval.assert_called_with()
+
     def test_drone_minimum_lifetime(self):
         self.site_adapter.drone_minimum_lifetime.return_value = None
         self.assertIsNone(self.site_agent.drone_minimum_lifetime())
diff --git a/tests/interfaces_t/test_siteadapter.py b/tests/interfaces_t/test_siteadapter.py
index c860b51..71eedd6 100644
--- a/tests/interfaces_t/test_siteadapter.py
+++ b/tests/interfaces_t/test_siteadapter.py
@@ -75,6 +75,24 @@ class TestSiteAdapter(TestCase):
                 ),
             )
 
+    def test_drone_heartbeat_interval(self):
+        self.assertEqual(self.site_adapter.drone_heartbeat_interval, 60)
+
+        # lru_cache needs to be cleared before manipulating site configuration
+        # noinspection PyUnresolvedReferences
+        SiteAdapter.site_configuration.fget.cache_clear()
+
+        self.config.Sites[0]["drone_heartbeat_interval"] = 10
+        self.assertEqual(self.site_adapter.drone_heartbeat_interval, 10)
+
+        # noinspection PyUnresolvedReferences
+        SiteAdapter.site_configuration.fget.cache_clear()
+
+        self.config.Sites[0]["drone_heartbeat_interval"] = -1
+        with self.assertRaises(ValidationError):
+            # noinspection PyStatementEffect
+            self.site_adapter.drone_heartbeat_interval
+
     def test_drone_minimum_lifetime(self):
         self.assertEqual(self.site_adapter.drone_minimum_lifetime, None)
 
@@ -194,6 +212,7 @@ class TestSiteAdapter(TestCase):
                 adapter="TestSite",
                 quota=1,
                 drone_minimum_lifetime=None,
+                drone_heartbeat_interval=60,
             ),
         )
 
@@ -209,6 +228,7 @@ class TestSiteAdapter(TestCase):
                 adapter="TestSite",
                 quota=inf,
                 drone_minimum_lifetime=None,
+                drone_heartbeat_interval=60,
             ),
         )
 
@@ -225,6 +245,7 @@ class TestSiteAdapter(TestCase):
                     adapter="TestSite",
                     quota=inf,
                     drone_minimum_lifetime=None,
+                    drone_heartbeat_interval=60,
                 ),
             )
 
diff --git a/tests/resources_t/test_drone.py b/tests/resources_t/test_drone.py
index 632fb59..4497069 100644
--- a/tests/resources_t/test_drone.py
+++ b/tests/resources_t/test_drone.py
@@ -33,6 +33,7 @@ class TestDrone(TestCase):
     def setUp(self) -> None:
         self.mock_site_agent.machine_meta_data = AttributeDict(Cores=8)
         self.mock_site_agent.drone_minimum_lifetime = None
+        self.mock_site_agent.drone_heartbeat_interval = 60
         self.mock_plugin = MagicMock(spec=Plugin)()
         self.mock_plugin.notify.return_value = async_return()
         self.drone = Drone(
@@ -53,10 +54,15 @@ class TestDrone(TestCase):
         self.drone.demand = 0
         self.assertEqual(self.drone.demand, 0)
 
+    def test_heartbeat_interval(self):
+        self.assertEqual(self.drone.heartbeat_interval, 60)
+        self.mock_site_agent.drone_heartbeat_interval = 10
+        self.assertEqual(self.drone.heartbeat_interval, 10)
+
     def test_life_time(self):
-        self.assertIsNone(self.drone.drone_minimum_lifetime, None)
+        self.assertIsNone(self.drone.minimum_lifetime, None)
         self.mock_site_agent.drone_minimum_lifetime = 3600
-        self.assertEqual(self.drone.drone_minimum_lifetime, 3600)
+        self.assertEqual(self.drone.minimum_lifetime, 3600)
 
     def test_maximum_demand(self):
         self.assertEqual(self.drone.maximum_demand, 8)
@@ -76,8 +82,9 @@ class TestDrone(TestCase):
     def test_site_agent(self):
         self.assertEqual(self.drone.site_agent, self.mock_site_agent)
 
-    @patch("tardis.resources.drone.asyncio.sleep", async_return)
-    def test_run(self):
+    @patch("tardis.resources.drone.asyncio.sleep")
+    def test_run(self, mocked_asyncio_sleep):
+        mocked_asyncio_sleep.side_effect = async_return
         mocked_down_state = MagicMock(spec=DownState)
         mocked_down_state.run.return_value = async_return()
 
@@ -89,9 +96,14 @@ class TestDrone(TestCase):
 
         run_async(self.drone.set_state, mocked_state)
         self.drone.demand = 8
+        self.mock_site_agent.drone_heartbeat_interval = 10
         with self.assertLogs(level=DEBUG):
             run_async(self.drone.run)
 
+        mocked_asyncio_sleep.assert_called_once_with(
+            self.mock_site_agent.drone_heartbeat_interval
+        )
+
         self.assertIsInstance(self.drone.state, DownState)
         self.assertEqual(self.drone.demand, 0)
 
diff --git a/tests/resources_t/test_dronestates.py b/tests/resources_t/test_dronestates.py
index 76fd2cc..ac62cab 100644
--- a/tests/resources_t/test_dronestates.py
+++ b/tests/resources_t/test_dronestates.py
@@ -55,7 +55,7 @@ class TestDroneStates(TestCase):
         )
         self.drone.demand = 8.0
         self.drone._supply = 8.0
-        self.drone.drone_minimum_lifetime = 3600
+        self.drone.minimum_lifetime = 3600
         self.drone.set_state.side_effect = partial(mock_set_state, self.drone)
         self.drone.site_agent.deploy_resource.return_value = async_return(
             return_value=AttributeDict()
@@ -218,7 +218,7 @@ class TestDroneStates(TestCase):
 
         # Test draining procedure due to exceeding life time of the drone
         self.drone.demand = 8.0
-        self.drone.drone_minimum_lifetime = 1
+        self.drone.minimum_lifetime = 1
         self.drone.state.return_value = AvailableState()
         run_async(self.drone.state.return_value.run, self.drone)
         self.assertIsInstance(self.drone.state, DrainState)