Avoid storing nodes in frontend - use UUIDs

[edan-bainglass]

We already use the process UUID in the results step to avoid DB session conflicts across threads. This PR extends this principle throughout the app, allowing a broader use of threads in the future.

In particular, the HasInputStructure and HasProcess mixins now operate with uuids. A property is introduced to each referencing the node. These properties are now cached using a new cache_per_thread decorator ensuring that each thread only loads the node once. Subsequent calls do not bother the DB. The cache is invalidated explicitly by the respective UUID.

Breaking change

Plugins need to switch referencing the old "input_structure" trait to "structure_uuid"

Update

Removing node caching - node loading is efficient

[codecov]

Codecov Report

❌ Patch coverage is 71.42857% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.25%. Comparing base (a0d8e9f) to head (688d215).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/aiidalab_qe/app/result/model.py	16.66%	5 Missing ⚠️
src/aiidalab_qe/app/submission/model.py	61.53%	5 Missing ⚠️
src/aiidalab_qe/common/mixins.py	79.16%	5 Missing ⚠️
src/aiidalab_qe/common/panel.py	25.00%	3 Missing ⚠️
src/aiidalab_qe/app/result/__init__.py	60.00%	2 Missing ⚠️
...aiidalab_qe/app/result/components/summary/model.py	71.42%	2 Missing ⚠️
src/aiidalab_qe/common/process/tree.py	90.00%	2 Missing ⚠️
...aiidalab_qe/app/configuration/advanced/advanced.py	0.00%	1 Missing ⚠️
...lab_qe/app/configuration/advanced/hubbard/model.py	0.00%	1 Missing ⚠️
...idalab_qe/app/result/components/summary/summary.py	50.00%	1 Missing ⚠️
... and 1 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1389      +/-   ##
==========================================
- Coverage   72.26%   72.25%   -0.01%     
==========================================
  Files         108      108              
  Lines        7254     7256       +2     
==========================================
+ Hits         5242     5243       +1     
- Misses       2012     2013       +1     

Flag	Coverage Δ
python-3.11	72.24% <70.40%> (-0.01%)	⬇️
python-3.9	72.28% <71.42%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[superstar54]

These properties are now cached using a new cache_per_thread decorator ensuring that each thread only loads the node once.

The cache_per_thread looks complex and hard to follow the logic. Could you explain in more detail? and why is this needed?

[edan-bainglass]

These properties are now cached using a new cache_per_thread decorator ensuring that each thread only loads the node once.

The cache_per_thread looks complex and hard to follow the logic. Could you explain in more detail? and why is this needed?

Sure. The motivation for the decorator is in the PR description. But just to reinforce this, consider the following from the results step:

    def _update_state(self):
        if not self._model.has_process:
            self.state = self.State.INIT
            self._update_controls()
            return

        if process_state := self._model.process.process_state:
            status = self._get_process_status(process_state.value)
        else:
            status = "Unknown"

        if process_state is ProcessState.CREATED:
            self.state = self.State.ACTIVE
        elif process_state in (
            ProcessState.RUNNING,
            ProcessState.WAITING,
        ):
            self.state = self.State.ACTIVE
            status = self._get_process_status("running")  # overwrite status
        elif process_state in (
            ProcessState.EXCEPTED,
            ProcessState.KILLED,
        ):
            self.state = self.State.FAIL
        elif self._model.process.is_failed:
            self.state = self.State.FAIL
        elif self._model.process.is_finished_ok:
            self.state = self.State.SUCCESS

        self._model.process_info = self.STATUS_TEMPLATE.format(status)

        self._update_controls()

Without caching HasProcess.process, each use of self._model.process will trigger a call to the DB. The cache ensures that each thread only fetches a given DB node once.

As for the implementation, the inner wrapper first checks if the thread already has a cache, otherwise it creates it. It then constructs a key from the class id, the method, the invalidator if provided (e.g., structure_uuid), and the arguments of the method. The key is added to the cache if it does not already exist. The value associated with the key is finally returned. Lastly, there is a check that allows also properties to be wrapped.

Input welcome of course, if you see a simpler way to implement it 🙂

Update

I updated the decorator implementation to allow decorating of plain function, but also for clarity, providing comments and extending the docstring. I hope this helps 🙂🙏

[edan-bainglass]

@superstar54 I retested this one this morning. The failed docker build is due to plugins having not yet updated w.r.t the "input_structure" trait changing to "structure_uuid" (in this PR). How should we handle this? Do we merge then update all plugins quickly, or should I leave behind a dummy trait (possible if necessary)?

[superstar54]

How should we handle this? Do we merge then update all plugins quickly, or should I leave behind a dummy trait (possible if necessary)?

I think there is no need to add a dummy trait. We can ignore the failed test this time. In the long term, we should create two Docker images with and without external plugins.

I'd like to look at the thread again. Let's discuss in the office later.

[edan-bainglass]

Note to self, loading nodes is quick

[superstar54]

LGTM!