-
Notifications
You must be signed in to change notification settings - Fork 322
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
Speed up SDV-Enterprise integration tests #2331
Changes from 4 commits
2ffad04
8840036
328a6bd
e07ad27
2cf0e93
0bd8621
e6e179b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
import contextlib | ||
import logging | ||
import shutil | ||
import tempfile | ||
from pathlib import Path | ||
|
||
import pandas as pd | ||
|
@@ -19,7 +20,9 @@ def get_sdv_logger_config(): | |
if (store_path / 'sdv_logger_config.yml').exists(): | ||
config_path = store_path / 'sdv_logger_config.yml' | ||
else: | ||
shutil.copyfile(config_path, store_path / 'sdv_logger_config.yml') | ||
tmp_path = tempfile.mktemp(dir=store_path, suffix='.yml') | ||
shutil.copyfile(config_path, tmp_path) | ||
shutil.move(tmp_path, store_path / 'sdv_logger_config.yml') | ||
Comment on lines
+23
to
+25
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change is required because multiple workers can access the logger at the same time ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this function can be called simultaneously by multiple workers, so while one of them was copying config_path into store_path, another one was resetting the store_path/config_path variables. |
||
|
||
with open(config_path, 'r') as f: | ||
logger_conf = yaml.safe_load(f) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of caching here, can we make a download_test_demo fixture in our test utils and just cache that one?