Disable caching for data #3807
Disable caching for data #3807
Conversation
Change the default for the _cachable class attribute to False in the Node class. This means it will be enabled only in the CalculationNode sub-classes.
Because the _cachable attribute is now set to False for Data nodes, the test for the '_store_from_cache' method failed because caching was not used. Instead of creating a valid cachable node (which is slightly complicated), we just call _store_from_cache directly to circumvent the 'is_valid_cache' and '_cachable' checks.
|
I had to adapt the test for Another option (which I also tried) would be using a |
greschd
requested review from
ltalirz and
sphuber
and removed request for
ltalirz
|
On the SQLA backend, |
| clone = data.clone().store() | ||
|
|
||
| clone = data.clone() | ||
| clone._store_from_cache(data, with_transaction=True) # pylint: disable=protected-access |
There was a problem hiding this comment.
It is a bit weird that we have a caching test for a Data node even though they are now explicitly not cacheable. Can we change this in a CalculationNode? Should be straightforward change and then you can revert to using with enable_caching()
There was a problem hiding this comment.
I did try that (see my comment above), but it becomes complicated trying to make sure that the CalculationNode fulfills all the requirements of the is_valid_cache check. I'd rather not rely on that specific behavior (as we may change the is_valid_cache check again).
There was a problem hiding this comment.
What if Nodes remain cacheable by default, but this is simply disabled at the Data level?
There was a problem hiding this comment.
This test is not possible with Node because they can not be stored.
Fixes #3802.
Change the default for the
_cachableclass attribute toFalsein theNodeclass. This means it will be enabled only in theCalculationNodesub-classes.