Repository: AgnostiqHQ/covalent
Pull request #1269 (2022-09-22 17:37:20)

Remove DB dependency from get_all_node_results
<!--
⚠️ Your pull request title should be short detailed and understandable for all.
⚠️ Increment the version number in the /VERSION file. If this is a bugfix, increment the patch version (the rightmost number), for example 0.18.2 becomes 0.18.3. If it's a new feature, increment the minor version (the middle number), for example 0.18.2 becomes 0.19.0.
⚠️ Add a note to /CHANGELOG.md with your version number and the date, summarizing the changes.
⚠️ If your pull request fixes an open issue, please link to the issue.
⚠️ Rebase the latest changes from the develop branch. Assuming origin points to https://github.com/AgnostiqHQ/covalent, you should run git rebase origin/develop.
-->

- [ ] I have added the tests to cover my changes.
- [x] I have updated the documentation and CHANGELOG accordingly.
- [x] I have read the CONTRIBUTING document.


FyzHsn: @scottwn These tests need to be updated, but otherwise LGTM. I'll approve this PR once the tests get fixed
scottwn: @FyzHsn I see in this commit you changed the assertion in `test_get_node_error` from `None` to `""`. Do you remember the reason for this? Can I change it back to `None`?
https://github.com/AgnostiqHQ/covalent/commit/67f5febd14bfeb4c9b115628c947c25b55743682#diff-011d90cacd1cb15b39a4a992517b7781ce550ee5a0a0b4401f7d97c58d7b8b0b
codecov[bot]: # [Codecov](https://codecov.io/gh/AgnostiqHQ/covalent/pull/1269?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ) Report
> Merging [#1269](https://codecov.io/gh/AgnostiqHQ/covalent/pull/1269?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ) (e4d7ca7) into [develop](https://codecov.io/gh/AgnostiqHQ/covalent/commit/b1a14b7d0bcd27bf8f59e13914bba9353200aac1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ) (b1a14b7) will **decrease** coverage by `4.38%`.
> The diff coverage is `94.11%`.

<details><summary>Additional details and impacted files</summary>


[![Impacted file tree graph](https://codecov.io/gh/AgnostiqHQ/covalent/pull/1269/graphs/tree.svg?width=650&height=150&src=pr&token=YGHCB3DE4P&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ)](https://codecov.io/gh/AgnostiqHQ/covalent/pull/1269?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ)

```diff
@@             Coverage Diff             @@
##           develop    #1269      +/-   ##
===========================================
- Coverage    86.45%   82.06%   -4.39%     
===========================================
  Files           59       62       +3     
  Lines         3816     3803      -13     
===========================================
- Hits          3299     3121     -178     
- Misses         517      682     +165     
```

| Flag | Coverage Δ | | *Carryforward flag |
|---|---|---|---|
| Dispatcher | `78.07% <91.60%> (+2.92%)` | :arrow_up: | Carriedforward from [4a40aa3](https://codecov.io/gh/AgnostiqHQ/covalent/commit/4a40aa3d96e6c1f588980fcd87b5351eb87ffe4b?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ) |
| Functional_Tests | `?` | | |
| SDK | `83.35% <95.09%> (+3.82%)` | :arrow_up: | |

*This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ) to find out more.

</details>

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 71929cab..4107b4f8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -31,6 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Updated `nbconvert` by dependabot
 - Split the `ConfigManager` into `Client` and `Server` components
 - Update the `set/get/update` config methods to distinguish between the client and server parts
+- `get_all_node_results()` uses in memory `Result` instead of DB
 - `get_all_node_outputs()` uses in memory Result instead of DB
 
 ### Removed
diff --git a/covalent/_results_manager/result.py b/covalent/_results_manager/result.py
index 3d4c3d59..3435ab8c 100644
--- a/covalent/_results_manager/result.py
+++ b/covalent/_results_manager/result.py
@@ -357,23 +357,10 @@ Node Outputs
         Returns:
             node_results: A list of dictionaries containing the result of every node execution.
         """
-        with workflow_db.session() as session:
-
-            lattice_id = (
-                session.query(models.Lattice)
-                .where(models.Lattice.dispatch_id == self.dispatch_id)
-                .first()
-                .id
-            )
-            electron_records = (
-                session.query(models.Electron)
-                .where(models.Electron.parent_lattice_id == lattice_id)
-                .all()
-            )
-            return [
-                self.get_node_result(node_id=electron.transport_graph_node_id)
-                for electron in electron_records
-            ]
+        return [
+            self.get_node_result(node_id=node_id)
+            for node_id in self._lattice.transport_graph._graph.nodes
+        ]
 
     def post_process(self):
 
@@ -406,25 +393,7 @@ Node Outputs
             The name of said node.
         """
 
-        with workflow_db.session() as session:
-
-            lattice_id = (
-                session.query(models.Lattice)
-                .where(models.Lattice.dispatch_id == self.dispatch_id)
-                .first()
-                .id
-            )
-            return (
-                session.query(models.Electron)
-                .where(
-                    and_(
-                        models.Electron.parent_lattice_id == lattice_id,
-                        models.Electron.transport_graph_node_id == node_id,
-                    )
-                )
-                .first()
-                .name
-            )
+        return self._lattice.transport_graph.get_node_value(node_id, "name")
 
     def _get_node_status(self, node_id: int) -> "Status":
         """
@@ -436,25 +405,7 @@ Node Outputs
         Returns:
             The status of said node.
         """
-
-        with workflow_db.session() as session:
-
-            lattice_id = (
-                session.query(models.Lattice)
-                .where(models.Lattice.dispatch_id == self.dispatch_id)
-                .first()
-            ).id
-            return (
-                session.query(models.Electron)
-                .where(
-                    and_(
-                        models.Electron.parent_lattice_id == lattice_id,
-                        models.Electron.transport_graph_node_id == node_id,
-                    )
-                )
-                .first()
-                .status
-            )
+        return self._lattice.transport_graph.get_node_value(node_id, "status")
 
     def _get_node_output(self, node_id: int) -> Any:
         """
@@ -515,28 +466,7 @@ Node Outputs
             error: The error of said node.
                    Will return None if no error occured in execution.
         """
-
-        with workflow_db.session() as session:
-
-            lattice_id = (
-                session.query(models.Lattice)
-                .where(models.Lattice.dispatch_id == self.dispatch_id)
-                .first()
-                .id
-            )
-            electron = (
-                session.query(models.Electron)
-                .where(
-                    (
-                        and_(
-                            models.Electron.parent_lattice_id == lattice_id,
-                            models.Electron.transport_graph_node_id == node_id,
-                        )
-                    )
-                )
-                .first()
-            )
-            return load_file(storage_path=electron.storage_path, filename=electron.stderr_filename)
+        return self._lattice.transport_graph.get_node_value(node_id, "error")
 
     def _update_node(
         self,


Test patch:
diff --git a/tests/covalent_tests/results_manager_tests/results_test.py b/tests/covalent_tests/results_manager_tests/results_test.py
index 4d612586..428c6918 100644
--- a/tests/covalent_tests/results_manager_tests/results_test.py
+++ b/tests/covalent_tests/results_manager_tests/results_test.py
@@ -332,13 +332,9 @@ def test_result_persist_rehydrate(test_db, result_1, mocker):
         assert tg_1.edges[e] == tg_2.edges[e]
 
 
-def test_get_node_error(test_db, result_1, mocker):
+def test_get_node_error(result_1):
     """Test result method to get the node error."""
-
-    mocker.patch("covalent._results_manager.write_result_to_db.workflow_db", test_db)
-    mocker.patch("covalent._results_manager.result.workflow_db", test_db)
-    result_1.persist()
-    assert result_1._get_node_error(node_id=0) == ""
+    assert not result_1._get_node_error(node_id=0)
 
 
 def test_get_node_value(test_db, result_1, mocker):