Skip to content

Commit

Permalink
Only instantiate the default cache if we are actually going to use it
Browse files Browse the repository at this point in the history
  • Loading branch information
Oddant1 committed May 29, 2024
1 parent 6ccbc15 commit 53c647c
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 24 deletions.
4 changes: 2 additions & 2 deletions q2cli/click/type.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ def type_expr(self):

def convert(self, value, param, ctx):
import qiime2.sdk.util
from q2cli.core.artifact_cache_global import USED_ARTIFACT_CACHE
from q2cli.core.artifact_cache_global import get_used_artifact_cache

with USED_ARTIFACT_CACHE:
with get_used_artifact_cache():
if value is None:
return None # Them's the rules

Expand Down
33 changes: 18 additions & 15 deletions q2cli/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,9 @@ def __call__(self, **kwargs):
from q2cli.util import (output_in_cache, _get_cache_path_and_key,
get_default_recycle_pool)
from q2cli.core.artifact_cache_global import (
USED_ARTIFACT_CACHE, unset_used_artifact_cache)
get_used_artifact_cache, unset_used_artifact_cache)

cache = get_used_artifact_cache()

output_dir = kwargs.pop('output_dir')
# If they gave us a cache and key combo as an output dir, we want to
Expand Down Expand Up @@ -443,18 +445,18 @@ def __call__(self, **kwargs):
value_ = value

if isinstance(value, list):
value_ = [USED_ARTIFACT_CACHE.process_pool.save(v)
value_ = [cache.process_pool.save(v)
for v in value]
elif isinstance(value, dict) or \
isinstance(value, ResultCollection):
value_ = {
k: USED_ARTIFACT_CACHE.process_pool.save(v)
k: cache.process_pool.save(v)
for k, v in value.items()}
elif isinstance(value, set):
value_ = set([USED_ARTIFACT_CACHE.process_pool.save(v)
value_ = set([cache.process_pool.save(v)
for v in value])
elif value is not None:
value_ = USED_ARTIFACT_CACHE.process_pool.save(value)
value_ = cache.process_pool.save(value)

arguments[key] = value_
else:
Expand All @@ -477,10 +479,10 @@ def __call__(self, **kwargs):
recycle_pool = default_pool

if recycle_pool is not None and recycle_pool != default_pool and \
recycle_pool not in USED_ARTIFACT_CACHE.get_pools():
recycle_pool not in cache.get_pools():
msg = ("The pool '%s' does not exist on the cache at '%s'. It "
"will be created." %
(recycle_pool, USED_ARTIFACT_CACHE.path))
(recycle_pool, cache.path))
click.echo(CONFIG.cfg_style('warning', msg))

# `qiime2.util.redirected_stdio` defaults to stdout/stderr when
Expand Down Expand Up @@ -519,10 +521,10 @@ def __call__(self, **kwargs):

with parallel_config:
results = self._execute_action(
action, arguments, recycle_pool)
action, arguments, cache, recycle_pool)
else:
results = self._execute_action(
action, arguments, recycle_pool)
action, arguments, cache, recycle_pool)
except Exception as e:
header = ('Plugin error from %s:'
% q2cli.util.to_cli_name(self.plugin['name']))
Expand Down Expand Up @@ -577,22 +579,23 @@ def __call__(self, **kwargs):
# the very end so if a failure happens during writing results we still
# have them
if recycle_pool == default_pool:
USED_ARTIFACT_CACHE.remove(recycle_pool)
cache.remove(recycle_pool)

# Set the USED_ARTIFACT_CACHE back to the default cache. This is mostly
# useful for the tests that invoke actions back to back to back without
# exiting the process
import time
print('sleep')
time.sleep(10)
unset_used_artifact_cache()

def _execute_action(self, action, arguments, recycle_pool=None):
from q2cli.core.artifact_cache_global import USED_ARTIFACT_CACHE

with USED_ARTIFACT_CACHE:
def _execute_action(self, action, arguments, cache, recycle_pool=None):
with cache:
if recycle_pool is None:
results = action(**arguments)
results = results._result()
else:
pool = USED_ARTIFACT_CACHE.create_pool(
pool = cache.create_pool(
key=recycle_pool, reuse=True)
with pool:
results = action(**arguments)
Expand Down
33 changes: 26 additions & 7 deletions q2cli/core/artifact_cache_global.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@
from qiime2.core.cache import Cache


# Init this to the default temp cache
USED_ARTIFACT_CACHE = Cache()
# Do not make this the default cache. If this is the default cache then we will
# instantiate the default cache when the module is imported which will write a
# process pool to the default cache which is undesirable if that isn't the
# cache we will be using for this action
_USED_ARTIFACT_CACHE = None


def set_used_artifact_cache(args):
Expand All @@ -21,7 +24,7 @@ def set_used_artifact_cache(args):
Parameters
----------
args : List[str]
The arguments provided on the cli to this QIIME 2 invocation
The arguments provided on the cli to this QIIME 2 invocation/
NOTES
-----
Expand All @@ -32,7 +35,7 @@ def set_used_artifact_cache(args):
"""
from q2cli.util import exit_with_error

global USED_ARTIFACT_CACHE
global _USED_ARTIFACT_CACHE

use_cache_idx = args.index('--use-cache')

Expand All @@ -51,12 +54,28 @@ def set_used_artifact_cache(args):
'not a path to an existing cache.')
exit_with_error(exc)

USED_ARTIFACT_CACHE = Cache(cache_path)
_USED_ARTIFACT_CACHE = Cache(cache_path)


def unset_used_artifact_cache():
"""Set the USED_ARTIFACT_CACHE back to the default cache.
"""
global USED_ARTIFACT_CACHE
global _USED_ARTIFACT_CACHE

USED_ARTIFACT_CACHE = Cache()
_USED_ARTIFACT_CACHE = None


def get_used_artifact_cache():
"""If the used cache has been set then return it otherwise return the
default cache. We use this getter because we don't want to instantiate the
default cache unless that is the cache we are using. This is because if we
instantiate the default cache we will put a process pool in it, and we want
to avoid that unless necessary.
Returns
-------
Cache
The default cache if the user didn't set a cache or the cache they set
if they did set one.
"""
return Cache() if _USED_ARTIFACT_CACHE is None else _USED_ARTIFACT_CACHE

0 comments on commit 53c647c

Please sign in to comment.