-
Notifications
You must be signed in to change notification settings - Fork 795
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
Data dir #994
Data dir #994
Changes from all commits
a09f286
bc14136
c99349f
d04e5f3
2d1b84c
4c9cd3e
64b9431
c900694
90acc46
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 |
---|---|---|
|
@@ -6,6 +6,7 @@ | |
from typing import Dict, List | ||
|
||
import monkey_island.cc.environment.server_config_generator as server_config_generator | ||
from monkey_island.cc.consts import DEFAULT_DATA_DIR | ||
from monkey_island.cc.environment.user_creds import UserCreds | ||
from monkey_island.cc.resources.auth.auth_user import User | ||
from monkey_island.cc.resources.auth.user_store import UserStore | ||
|
@@ -18,6 +19,7 @@ def __init__(self, file_path): | |
self.deployment = None | ||
self.user_creds = None | ||
self.aws = None | ||
self.data_dir = None | ||
|
||
self._load_from_file(self._server_config_path) | ||
|
||
|
@@ -26,7 +28,7 @@ def _load_from_file(self, file_path): | |
|
||
if not Path(file_path).is_file(): | ||
server_config_generator.create_default_config_file(file_path) | ||
with open(file_path, 'r') as f: | ||
with open(file_path, "r") as f: | ||
config_content = f.read() | ||
|
||
self._load_from_json(config_content) | ||
|
@@ -37,22 +39,29 @@ def _load_from_json(self, config_json: str) -> EnvironmentConfig: | |
|
||
def _load_from_dict(self, dict_data: Dict): | ||
user_creds = UserCreds.get_from_dict(dict_data) | ||
aws = dict_data['aws'] if 'aws' in dict_data else None | ||
aws = dict_data["aws"] if "aws" in dict_data else None | ||
data_dir = ( | ||
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. Var name should be more explicit if possible |
||
dict_data["data_dir"] if "data_dir" in dict_data else DEFAULT_DATA_DIR | ||
) | ||
|
||
self.server_config = dict_data['server_config'] | ||
self.deployment = dict_data['deployment'] | ||
self.server_config = dict_data["server_config"] | ||
self.deployment = dict_data["deployment"] | ||
self.user_creds = user_creds | ||
self.aws = aws | ||
self.data_dir = data_dir | ||
|
||
def save_to_file(self): | ||
with open(self._server_config_path, 'w') as f: | ||
with open(self._server_config_path, "w") as f: | ||
f.write(json.dumps(self.to_dict(), indent=2)) | ||
|
||
def to_dict(self) -> Dict: | ||
config_dict = {'server_config': self.server_config, | ||
'deployment': self.deployment} | ||
config_dict = { | ||
"server_config": self.server_config, | ||
"deployment": self.deployment, | ||
"data_dir": self.data_dir, | ||
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. Same complaint, this should be more specific. In the context of environment config, this could be something like |
||
} | ||
if self.aws: | ||
config_dict.update({'aws': self.aws}) | ||
config_dict.update({"aws": self.aws}) | ||
config_dict.update(self.user_creds.to_dict()) | ||
return config_dict | ||
|
||
|
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.
Term "Data" is misleading. This is a path to island codebase, that's what I'd call this.
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.
By default it is, only because that's what we're doing today. We need to move away from storing data generated at runtime alongside our code. Since we're so close to a release, I don't want to change that right now, except when running from an AppImage.