-
Notifications
You must be signed in to change notification settings - Fork 180
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
Workspace context manager #148
Changes from 4 commits
df0e579
e04c2e0
a58c28d
ceb5b7d
e2e00ac
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 |
---|---|---|
|
@@ -4,12 +4,7 @@ | |
"""Director clients module.""" | ||
|
||
import logging | ||
import os | ||
import shutil | ||
import time | ||
from datetime import datetime | ||
from subprocess import check_call | ||
from sys import executable | ||
|
||
import grpc | ||
|
||
|
@@ -69,67 +64,28 @@ def report_shard_info(self, shard_descriptor) -> bool: | |
acknowledgement = self.stub.AcknowledgeShard(shard_info) | ||
return acknowledgement.accepted | ||
|
||
def get_experiment_data(self): | ||
"""Get an experiment data from the director.""" | ||
def wait_experiment(self): | ||
"""Wait an experiment data from the director.""" | ||
Comment on lines
+67
to
+68
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. Maybe |
||
logger.info('Send WaitExperiment request') | ||
response_iter = self.stub.WaitExperiment(self._get_experiment_data()) | ||
logger.info('WaitExperiment response has received') | ||
response = next(response_iter) | ||
experiment_name = response.experiment_name | ||
if not experiment_name: | ||
raise Exception('No experiment') | ||
|
||
return experiment_name | ||
|
||
def get_experiment_data(self, experiment_name): | ||
"""Get an experiment data from the director.""" | ||
logger.info(f'Request experiment {experiment_name}') | ||
request = director_pb2.GetExperimentDataRequest( | ||
experiment_name=experiment_name, | ||
collaborator_name=self.shard_name | ||
) | ||
response_iter = self.stub.GetExperimentData(request) | ||
data_stream = self.stub.GetExperimentData(request) | ||
|
||
self.create_workspace(experiment_name, response_iter) | ||
|
||
return experiment_name | ||
|
||
@staticmethod | ||
def remove_workspace(experiment_name): | ||
"""Remove the workspace.""" | ||
shutil.rmtree(experiment_name, ignore_errors=True) | ||
|
||
@staticmethod | ||
def create_workspace(experiment_name, response_iter): | ||
"""Create a collaborator workspace for the experiment.""" | ||
if os.path.exists(experiment_name): | ||
shutil.rmtree(experiment_name) | ||
os.makedirs(experiment_name) | ||
|
||
arch_name = f'{experiment_name}/{experiment_name}' + '.zip' | ||
logger.info(f'arch_name: {arch_name}') | ||
with open(arch_name, 'wb') as content_file: | ||
for response in response_iter: | ||
logger.info(f'Size: {response.size}') | ||
if response.size == len(response.npbytes): | ||
content_file.write(response.npbytes) | ||
else: | ||
raise Exception('Broken archive') | ||
|
||
shutil.unpack_archive(arch_name, experiment_name) | ||
os.remove(arch_name) | ||
|
||
requirements_filename = f'./{experiment_name}/requirements.txt' | ||
|
||
if os.path.isfile(requirements_filename): | ||
attempts = 3 | ||
for _ in range(attempts): | ||
try: | ||
check_call([ | ||
executable, '-m', 'pip', 'install', '-r', requirements_filename], | ||
shell=False) | ||
except Exception as exc: | ||
logger.error(f'Failed to install requirements: {exc}') | ||
time.sleep(3) | ||
else: | ||
break | ||
else: | ||
logger.error('No ' + requirements_filename + ' file found.') | ||
return data_stream | ||
|
||
def _get_experiment_data(self): | ||
"""Generate the experiment data request.""" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
# Copyright (C) 2020-2021 Intel Corporation | ||
# SPDX-License-Identifier: Apache-2.0 | ||
|
||
"""Workspace utils module.""" | ||
|
||
import logging | ||
import os | ||
import shutil | ||
import sys | ||
import time | ||
from pathlib import Path | ||
from subprocess import check_call | ||
from sys import executable | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
||
class ExperimentWorkspace: | ||
"""Experiment workspace context manager.""" | ||
|
||
def __init__( | ||
self, | ||
experiment_name: str, | ||
data_file_path: Path, | ||
is_install_requirements: bool = False | ||
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. is_install_requirements -> install_requirements 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. It's boolean flag. It will be strange: if self.install_requirements:
self._install_requirements() Probably 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. I also think that |
||
) -> None: | ||
"""Initialize workspace context manager.""" | ||
self.experiment_name = experiment_name | ||
self.data_file_path = data_file_path | ||
self.cwd = os.getcwd() | ||
self.experiment_work_dir = f'{self.cwd}/{self.experiment_name}' | ||
self.is_install_requirements = is_install_requirements | ||
|
||
def _install_requirements(self): | ||
"""Install experiment requirements.""" | ||
requirements_filename = f'{self.experiment_work_dir}/requirements.txt' | ||
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. Maybe better to use 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 variable is used in 3 places in this method, some of them are converted to a string, and some must be converted to a string manually. None of them use type Path. |
||
|
||
if os.path.isfile(requirements_filename): | ||
attempts = 10 | ||
for _ in range(attempts): | ||
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. @aleksandr-mokrov Shouldn't we raise an exception when all attempts to install the requirements failed? |
||
try: | ||
check_call([ | ||
executable, '-m', 'pip', 'install', '-r', requirements_filename], | ||
shell=False) | ||
except Exception as exc: | ||
logger.error(f'Failed to install requirements: {exc}') | ||
# It's a workaround for cases when collaborators run | ||
# in common virtual environment | ||
time.sleep(5) | ||
else: | ||
break | ||
else: | ||
logger.error('No ' + requirements_filename + ' file found.') | ||
|
||
def __enter__(self): | ||
"""Create a collaborator workspace for the experiment.""" | ||
if os.path.exists(self.experiment_work_dir): | ||
shutil.rmtree(self.experiment_work_dir) | ||
os.makedirs(self.experiment_work_dir) | ||
|
||
shutil.unpack_archive(self.data_file_path, self.experiment_work_dir, format='zip') | ||
|
||
if self.is_install_requirements: | ||
self._install_requirements() | ||
|
||
os.chdir(self.experiment_work_dir) | ||
|
||
# This is needed for python module finder | ||
sys.path.append(self.experiment_work_dir) | ||
|
||
def __exit__(self, exc_type, exc_value, traceback): | ||
"""Remove the workspace.""" | ||
os.chdir(self.cwd) | ||
shutil.rmtree(self.experiment_name, ignore_errors=True) | ||
if self.experiment_work_dir in sys.path: | ||
sys.path.remove(self.experiment_work_dir) | ||
os.remove(self.data_file_path) |
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.
Maybe rename
is_install_requirements
=>requirements
in order to avoid so long line.