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

Replace remote S3 paths with local paths for test_snake dryruns. #915

Merged
merged 3 commits into from
Mar 3, 2019

Conversation

yesimon
Copy link
Contributor

@yesimon yesimon commented Jan 24, 2019

Save time querying S3 remote file presence during dryruns. Since the files are not specified
directly in config (they get operations to add suffixes in the rules), need to create a fake tree
of files (or hardcode the paths to rewrite).

yesimon added a commit that referenced this pull request Mar 1, 2019
Add support for krakenuniq, kaiju to replace kraken and diamond in
default pipelines.

Both rely on customized krakenuniq/kaiju packages for custom
functionality, like loading database
from pipe or printing out all taxa hits (kaiju).

Krona fixed to use hit counts as scores in report files directly,
instead of counting the
individual read classifications (which takes a long time).

Includes changes from #915 and also some pytest deprecation fixes:
s/log.warn(/log.warning(/, runslow command line configuration.
Save time querying S3 remote file presence during dryruns. Since the files are not specified
directly in config (they get operations to add suffixes in the rules), need to create a fake tree
of files (or hardcode the paths to rewrite).
Copy link
Member

@tomkinsc tomkinsc left a comment

Choose a reason for hiding this comment

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

This should certainly help speed up the tests. Looks good, other than the minor feedback about touch_p (and maybe creating the empty placeholder files dynamically).

util/file.py Outdated Show resolved Hide resolved

def translate_remote_s3(uri):
remote_path = uri[5:]
fake_s3_root = os.path.join(util.file.get_project_path(), 'test', 'input', 's3')
Copy link
Member

Choose a reason for hiding this comment

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

Maybe all of the empty placeholder files should be made dynamically as part of the tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's hard to determine what the filenames need to be dynamically. Either need to store a list of filenames to then create at runtime or create empty files - I'm ambivalent about these two.

@yesimon yesimon requested a review from tomkinsc March 3, 2019 00:27
@yesimon yesimon merged commit 157cd7b into master Mar 3, 2019
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.

2 participants