Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Override cache dir if PYENSEMBL_CACHE_DIR is set #156

Merged
merged 1 commit into from
Jul 22, 2016
Merged

Conversation

armish
Copy link
Contributor

@armish armish commented Jul 21, 2016

so that we can write the cache to a shared directory when running analyses on multi-node clusters.

Context: hammerlab/biokepi#268

cc: @arahuja and @seb


This change is Reviewable

@@ -111,8 +112,12 @@ def __init__(
annotation_name=annotation_name,
annotation_version=annotation_version)

self._cache_directory_path = datacache.get_data_dir(
subdir=self.cache_subdirectory)
if environ.get(ENV_KEY) is None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct — but I wanted to make this more explicit as it is not apparent from the function call what will happen if you pass both variables.

Actually, maybe adding a comment there to explain this might be better. Let me do that 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@armish armish force-pushed the custom-cache-dir branch 2 times, most recently from 2847d76 to df22255 Compare July 21, 2016 21:11
@coveralls
Copy link

coveralls commented Jul 21, 2016

Coverage Status

Coverage increased (+0.01%) to 80.039% when pulling df22255 on custom-cache-dir into 3108c6e on master.

@coveralls
Copy link

coveralls commented Jul 21, 2016

Coverage Status

Coverage increased (+0.01%) to 80.039% when pulling df22255 on custom-cache-dir into 3108c6e on master.

@coveralls
Copy link

coveralls commented Jul 21, 2016

Coverage Status

Coverage increased (+0.01%) to 80.039% when pulling df22255 on custom-cache-dir into 3108c6e on master.

self._cache_directory_path = datacache.get_data_dir(
subdir=self.cache_subdirectory)
subdir=self.cache_subdirectory,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: indentation

@tavinathanson
Copy link
Contributor

LGTM!

from os.path import join, exists, split, abspath
from shutil import copy2, rmtree
import logging

import datacache

CACHE_BASE_SUBDIR = "pyensembl"
ENV_KEY = "PYENSEMBL_CACHE_DIR"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: might call this something a bit clearer and non-abbreviated like CACHE_DIRECTORY_ENVIRONMENT_VARIABLE

@armish
Copy link
Contributor Author

armish commented Jul 22, 2016

thanks all! Addressed your comments in my amended commit. Will merge and release on 💚

@iskandr
Copy link
Contributor

iskandr commented Jul 22, 2016

Fixes #145

@coveralls
Copy link

coveralls commented Jul 22, 2016

Coverage Status

Coverage increased (+0.3%) to 80.363% when pulling 75ffb6a on custom-cache-dir into 3108c6e on master.

@armish armish merged commit 30774ce into master Jul 22, 2016
@armish armish deleted the custom-cache-dir branch July 22, 2016 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants