Skip to content

Commit

Permalink
use AppConfig initializer with file parameter
Browse files Browse the repository at this point in the history
  • Loading branch information
Bento007 committed Nov 15, 2022
1 parent 3f42e57 commit 2001c87
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 34 deletions.
10 changes: 7 additions & 3 deletions server/common/config/app_config.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import copy
import logging

import yaml
from flatten_dict import unflatten as _unflatten, flatten as _flatten

Expand All @@ -18,9 +20,8 @@ def unflatten(dictionary: dict) -> dict:
class AppConfig(object):
"""
AppConfig stores all the configuration for cellxgene.
AppConfig contains one or more DatasetConfig(s) and one ServerConfig.
The server_config contains attributes that refer to the server process as a whole.
The default_dataset_config refers to attributes that are associated with the features and
The self.server contains attributes that refer to the server process as a whole.
The self.default_dataset refers to attributes that are associated with the features and
presentations of a dataset.
The dataset config attributes can be overridden depending on the url by which the
dataset was accessed. These are stored in dataroot_config.
Expand All @@ -33,6 +34,7 @@ def __init__(self, config_file_path: str = None):
self.default_config: dict = get_default_config()
self.config: dict = copy.deepcopy(self.default_config)
self.valid = False
self.dataroot_config = {}
if config_file_path:
self.update_from_config_file(config_file_path)

Expand Down Expand Up @@ -83,6 +85,7 @@ def update_config(self, **kw):
)
for value in self.dataroot_config.values():
value = value.update(**new_config["default_dataset"])
logging.info("Configuration updated")

def _open_config_file(self, config_file: str):
try:
Expand Down Expand Up @@ -115,6 +118,7 @@ def complete_config(self):
self.config = self.validate_config(self.config)
self.handle_adaptor()
self.handle_data_source()
logging.info("Configuration complete.")

def get_dataset_config(self, dataroot_key: str) -> dict:
return self.dataroot_config.get(dataroot_key, self.default_dataset)
Expand Down
15 changes: 5 additions & 10 deletions server/ecs/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,18 +140,14 @@ def get_csp_hashes(app, app_config):


try:
app_config = AppConfig()

has_config = False
app_config = False
# config file: look first for "config.yaml" in the current working directory
config_file = "config.yaml"
config_location = DataLocator(config_file)
if config_location.exists():
with config_location.local_handle() as lh:
logging.info(f"Configuration from {config_file}")
app_config.update_from_config_file(lh)
has_config = True

app_config = AppConfig(lh)
else:
# config file: second, use the CXG_CONFIG_FILE
config_file = os.getenv("CXG_CONFIG_FILE")
Expand All @@ -161,13 +157,12 @@ def get_csp_hashes(app, app_config):
if config_location.exists():
with config_location.local_handle() as lh:
logging.info(f"Configuration from {config_file}")
app_config.update_from_config_file(lh)
has_config = True
app_config = AppConfig(lh)
else:
logging.critical(f"Configuration file not found {config_file}")
sys.exit(1)

if not has_config:
if not app_config:
logging.critical("No config file found")
sys.exit(1)

Expand All @@ -177,7 +172,7 @@ def get_csp_hashes(app, app_config):
app_config.update_server_config(multi_dataset__dataroot=dataroot)

# complete config
app_config.complete_config(logging.info)
app_config.complete_config()

server = WSGIServer(app_config)
debug = False
Expand Down
9 changes: 3 additions & 6 deletions server/tests/unit/common/config/test_app_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ def get_config(self, **kwargs):
file_name = self.custom_app_config(
dataroot=f"{FIXTURES_ROOT}", config_file_name=self.config_file_name, **kwargs
)
config = AppConfig()
config.update_from_config_file(file_name)
config = AppConfig(file_name)
return config

def test_get_default_config_correctly_reads_default_config_file(self):
Expand Down Expand Up @@ -82,8 +81,7 @@ def test_configfile_no_dataset_section(self):
"""
fconfig.write(config)

app_config = AppConfig()
app_config.update_from_config_file(configfile)
app_config = AppConfig(configfile)
server_changes = self.compare_configs(app_config, default_config)
self.assertCountEqual(
server_changes,
Expand All @@ -106,8 +104,7 @@ def test_configfile_no_server_section(self):
"""
fconfig.write(config)

app_config = AppConfig()
app_config.update_from_config_file(configfile)
app_config = AppConfig(configfile)
changes = self.compare_configs(app_config, default_config)
self.assertCountEqual(changes, [('default_dataset__app__about_legal_tos', 'expected_value', None),
('server__app__port', 5005, None),
Expand Down
3 changes: 1 addition & 2 deletions server/tests/unit/common/config/test_base_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ def get_config(self, **kwargs):
file_name = self.custom_app_config(
config_file_name=self.config_file_name, **kwargs
)
config = AppConfig()
config.update_from_config_file(file_name)
config = AppConfig(file_name)
return config

def test_mapping_creation_returns_map_of_server_and_dataset_config(self):
Expand Down
4 changes: 1 addition & 3 deletions server/tests/unit/common/config/test_dataset_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,7 @@ def test_configfile_with_specialization(self):
"""
fconfig.write(config)

app_config = AppConfig()
app_config.update_from_config_file(configfile)
app_config = AppConfig(configfile)

# test config from specialization
self.assertEqual(app_config.server__multi_dataset__dataroots["test"]["base_url"], "test")
15 changes: 5 additions & 10 deletions server/tests/unit/common/config/test_server_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ def get_config(self, **kwargs):
file_name = self.custom_app_config(
dataroot=f"{FIXTURES_ROOT}", config_file_name=self.config_file_name, **kwargs
)
config = AppConfig()
config.update_from_config_file(file_name)
config = AppConfig(file_name)
return config

def test_init_raises_error_if_default_config_is_invalid(self):
Expand All @@ -60,9 +59,8 @@ def test_handle_data_locator_works_for_default_types(self, mock_discover_region_
file_name = self.custom_app_config(
dataroots=dataroots, config_file_name=self.config_file_name, data_locator_region_name="true"
)
config = AppConfig()
with self.assertRaises(ConfigurationError):
config.update_from_config_file(file_name)
config = AppConfig(file_name)

@patch("server.common.config.config_model.discover_s3_region_name")
def test_handle_data_locator_can_read_from_dataroot(self, mock_discover_region_name):
Expand All @@ -74,8 +72,7 @@ def test_handle_data_locator_can_read_from_dataroot(self, mock_discover_region_n
file_name = self.custom_app_config(
dataroots=dataroots, config_file_name=self.config_file_name, data_locator_region_name="true"
)
config = AppConfig()
config.update_from_config_file(file_name)
config = AppConfig(file_name)
self.assertEqual(config.server__data_locator__s3_region_name, "us-west-2")
mock_discover_region_name.assert_called_once_with("s3://hosted-cellxgene-dev")

Expand All @@ -90,8 +87,7 @@ def test_handle_data_source__errors_when_passed_zero(self):
)

file_name = self.custom_app_config(config_file_name="zero_roots.yml")
config = AppConfig()
config.update_from_config_file(file_name)
config = AppConfig(file_name)
with self.subTest("zero roots"):
with self.assertRaises(ConfigurationError):
config.handle_data_source()
Expand Down Expand Up @@ -221,8 +217,7 @@ def test_handle_adaptor(self, mock_tiledb_context):
cxg_tile_cache_size=10,
cxg_tiledb_py_init_buffer_size=10,
)
config = AppConfig()
config.update_from_config_file(custom_config)
config = AppConfig(custom_config)
config.handle_adaptor()
mock_tiledb_context.assert_called_once_with(
{
Expand Down

0 comments on commit 2001c87

Please sign in to comment.