Skip to content

Commit

Permalink
Merge pull request #4800 from stsewd/remove-support-for-mult-confs
Browse files Browse the repository at this point in the history 
Remove support for multiple configurations in one file
  • Loading branch information
@ericholscher
ericholscher authored Nov 9, 2018
2 parents 3930383 + 178d744 commit 1648006
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 137 deletions.
56 changes: 16 additions & 40 deletions readthedocs/config/config.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
'ConfigError',
'ConfigOptionNotSupportedError',
'InvalidConfig',
'ProjectConfig',
)

ALL = 'all'
Expand Down Expand Up @@ -110,12 +109,10 @@ class InvalidConfig(ConfigError):

message_template = 'Invalid "{key}": {error}'

def __init__(self, key, code, error_message, source_file=None,
source_position=None):
def __init__(self, key, code, error_message, source_file=None):
self.key = key
self.code = code
self.source_file = source_file
self.source_position = source_position
message = self.message_template.format(
key=key,
code=code,
Expand Down Expand Up @@ -149,11 +146,10 @@ class BuildConfigBase(object):
]
version = None

def __init__(self, env_config, raw_config, source_file, source_position):
def __init__(self, env_config, raw_config, source_file):
self.env_config = env_config
self.raw_config = raw_config
self.source_file = source_file
self.source_position = source_position
if os.path.isdir(self.source_file):
self.base_path = self.source_file
else:
Expand All @@ -165,10 +161,7 @@ def __init__(self, env_config, raw_config, source_file, source_position):
def error(self, key, message, code):
"""Raise an error related to ``key``."""
if not os.path.isdir(self.source_file):
source = '{file} [{pos}]'.format(
file=os.path.relpath(self.source_file, self.base_path),
pos=self.source_position,
)
source = os.path.relpath(self.source_file, self.base_path)
error_message = '{source}: {message}'.format(
source=source,
message=message,
Expand All @@ -180,7 +173,6 @@ def error(self, key, message, code):
code=code,
error_message=error_message,
source_file=self.source_file,
source_position=self.source_position,
)

@contextmanager
Expand All @@ -194,7 +186,6 @@ def catch_validation_error(self, key):
code=error.code,
error_message=str(error),
source_file=self.source_file,
source_position=self.source_position,
)

def pop(self, name, container, default, raise_ex):
Expand Down Expand Up @@ -1058,16 +1049,6 @@ def submodules(self):
return Submodules(**self._config['submodules'])


class ProjectConfig(list):

"""Wrapper for multiple build configs."""

def validate(self):
"""Validates each configuration build."""
for build in self:
build.validate()


def load(path, env_config):
"""
Load a project configuration and the top-most build config for a given path.
Expand All @@ -1083,10 +1064,9 @@ def load(path, env_config):
'No configuration file found',
code=CONFIG_REQUIRED
)
build_configs = []
with open(filename, 'r') as configuration_file:
try:
configs = parse(configuration_file.read())
config = parse(configuration_file.read())
except ParseError as error:
raise ConfigError(
'Parse error in {filename}: {message}'.format(
Expand All @@ -1095,23 +1075,19 @@ def load(path, env_config):
),
code=CONFIG_SYNTAX_INVALID,
)
for i, config in enumerate(configs):
allow_v2 = env_config.get('allow_v2')
if allow_v2:
version = config.get('version', 1)
else:
version = 1
build_config = get_configuration_class(version)(
env_config,
config,
source_file=filename,
source_position=i,
)
build_configs.append(build_config)
allow_v2 = env_config.get('allow_v2')
if allow_v2:
version = config.get('version', 1)
else:
version = 1
build_config = get_configuration_class(version)(
env_config,
config,
source_file=filename,
)

project_config = ProjectConfig(build_configs)
project_config.validate()
return project_config
build_config.validate()
return build_config


def get_configuration_class(version):
Expand Down
15 changes: 7 additions & 8 deletions readthedocs/config/parser.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,17 @@ class ParseError(Exception):

def parse(stream):
"""
Take file-like object and return a list of project configurations.
Take file-like object and return a project configuration.
The files need be valid YAML and only contain mappings as documents.
The file need be valid YAML and only contain mappings as document.
Everything else raises a ``ParseError``.
"""
try:
configs = list(yaml.safe_load_all(stream))
config = yaml.safe_load(stream)
except yaml.YAMLError as error:
raise ParseError('YAML: {message}'.format(message=error))
if not configs:
if not isinstance(config, dict):
raise ParseError('Expected mapping')
if not config:
raise ParseError('Empty config')
for config in configs:
if not isinstance(config, dict):
raise ParseError('Expected mapping')
return configs
return config
71 changes: 9 additions & 62 deletions readthedocs/config/tests/test_config.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
ConfigError,
ConfigOptionNotSupportedError,
InvalidConfig,
ProjectConfig,
load,
)
from readthedocs.config.config import (
Expand Down Expand Up @@ -81,13 +80,11 @@
}


def get_build_config(config, env_config=None, source_file='readthedocs.yml',
source_position=0):
def get_build_config(config, env_config=None, source_file='readthedocs.yml'):
return BuildConfigV1(
env_config or {},
config,
source_file=source_file,
source_position=source_position,
)


Expand Down Expand Up @@ -130,10 +127,7 @@ def test_load_empty_config_file(tmpdir):
def test_minimal_config(tmpdir):
apply_fs(tmpdir, minimal_config_dir)
base = str(tmpdir)
config = load(base, env_config)
assert isinstance(config, ProjectConfig)
assert len(config) == 1
build = config[0]
build = load(base, env_config)
assert isinstance(build, BuildConfigV1)


Expand All @@ -144,10 +138,7 @@ def test_load_version1(tmpdir):
''')
})
base = str(tmpdir)
config = load(base, get_env_config({'allow_v2': True}))
assert isinstance(config, ProjectConfig)
assert len(config) == 1
build = config[0]
build = load(base, get_env_config({'allow_v2': True}))
assert isinstance(build, BuildConfigV1)


Expand All @@ -158,10 +149,7 @@ def test_load_version2(tmpdir):
''')
})
base = str(tmpdir)
config = load(base, get_env_config({'allow_v2': True}))
assert isinstance(config, ProjectConfig)
assert len(config) == 1
build = config[0]
build = load(base, get_env_config({'allow_v2': True}))
assert isinstance(build, BuildConfigV2)


Expand All @@ -182,31 +170,18 @@ def test_yaml_extension(tmpdir):
apply_fs(tmpdir, yaml_extension_config_dir)
base = str(tmpdir)
config = load(base, env_config)
assert len(config) == 1
assert isinstance(config, BuildConfigV1)


def test_build_config_has_source_file(tmpdir):
base = str(apply_fs(tmpdir, minimal_config_dir))
build = load(base, env_config)[0]
build = load(base, env_config)
assert build.source_file == os.path.join(base, 'readthedocs.yml')
assert build.source_position == 0


def test_build_config_has_source_position(tmpdir):
base = str(apply_fs(tmpdir, multiple_config_dir))
builds = load(base, env_config)
assert len(builds) == 2
first, second = filter(
lambda b: not b.source_file.endswith('nested/readthedocs.yml'),
builds,
)
assert first.source_position == 0
assert second.source_position == 1


def test_build_config_has_list_with_single_empty_value(tmpdir):
base = str(apply_fs(tmpdir, config_with_explicit_empty_list))
build = load(base, env_config)[0]
build = load(base, env_config)
assert isinstance(build, BuildConfigV1)
assert build.formats == []

Expand All @@ -216,7 +191,6 @@ def test_config_requires_name():
{'output_base': ''},
{},
source_file='readthedocs.yml',
source_position=0,
)
with raises(InvalidConfig) as excinfo:
build.validate()
Expand All @@ -229,7 +203,6 @@ def test_build_requires_valid_name():
{'output_base': ''},
{'name': 'with/slashes'},
source_file='readthedocs.yml',
source_position=0,
)
with raises(InvalidConfig) as excinfo:
build.validate()
Expand Down Expand Up @@ -553,7 +526,6 @@ def test_valid_build_config():
env_config,
minimal_config,
source_file='readthedocs.yml',
source_position=0,
)
build.validate()
assert build.name == 'docs'
Expand All @@ -575,7 +547,6 @@ def test_it_validates_to_abspath(self, tmpdir):
get_env_config(),
{'base': '../docs'},
source_file=source_file,
source_position=0,
)
build.validate()
assert build.base == str(tmpdir.join('docs'))
Expand All @@ -596,7 +567,6 @@ def test_it_fails_if_base_is_not_a_string(self, tmpdir):
get_env_config(),
{'base': 1},
source_file=str(tmpdir.join('readthedocs.yml')),
source_position=0,
)
with raises(InvalidConfig) as excinfo:
build.validate()
Expand All @@ -609,7 +579,6 @@ def test_it_fails_if_base_does_not_exist(self, tmpdir):
get_env_config(),
{'base': 'docs'},
source_file=str(tmpdir.join('readthedocs.yml')),
source_position=0,
)
with raises(InvalidConfig) as excinfo:
build.validate()
Expand All @@ -625,7 +594,6 @@ def test_it_fails_if_build_is_invalid_option(self, tmpdir):
get_env_config(),
{'build': {'image': 3.0}},
source_file=str(tmpdir.join('readthedocs.yml')),
source_position=0,
)
with raises(InvalidConfig) as excinfo:
build.validate()
Expand All @@ -641,7 +609,6 @@ def test_it_fails_on_python_validation(self, tmpdir):
'python': {'version': '3.3'},
},
source_file=str(tmpdir.join('readthedocs.yml')),
source_position=0,
)
build.validate_build()
with raises(InvalidConfig) as excinfo:
Expand All @@ -658,7 +625,6 @@ def test_it_works_on_python_validation(self, tmpdir):
'python': {'version': '3.3'},
},
source_file=str(tmpdir.join('readthedocs.yml')),
source_position=0,
)
build.validate_build()
build.validate_python()
Expand All @@ -669,7 +635,6 @@ def test_it_works(self, tmpdir):
get_env_config(),
{'build': {'image': 'latest'}},
source_file=str(tmpdir.join('readthedocs.yml')),
source_position=0,
)
build.validate()
assert build.build.image == 'readthedocs/build:latest'
Expand All @@ -680,7 +645,6 @@ def test_default(self, tmpdir):
get_env_config(),
{},
source_file=str(tmpdir.join('readthedocs.yml')),
source_position=0,
)
build.validate()
assert build.build.image == 'readthedocs/build:2.0'
Expand All @@ -696,7 +660,6 @@ def test_it_priorities_image_from_env_config(self, tmpdir, image):
get_env_config({'defaults': defaults}),
{'build': {'image': 'latest'}},
source_file=str(tmpdir.join('readthedocs.yml')),
source_position=0,
)
build.validate()
assert build.build.image == image
Expand Down Expand Up @@ -786,7 +749,6 @@ def test_build_validate_calls_all_subvalidators(tmpdir):
{},
{},
source_file=str(tmpdir.join('readthedocs.yml')),
source_position=0,
)
with patch.multiple(
BuildConfigV1,
Expand All @@ -802,20 +764,6 @@ def test_build_validate_calls_all_subvalidators(tmpdir):
BuildConfigV1.validate_output_base.assert_called_with()


def test_validate_project_config():
with patch.object(BuildConfigV1, 'validate') as build_validate:
project = ProjectConfig([
BuildConfigV1(
env_config,
minimal_config,
source_file='readthedocs.yml',
source_position=0,
),
])
project.validate()
assert build_validate.call_count == 1


def test_load_calls_validate(tmpdir):
apply_fs(tmpdir, minimal_config_dir)
base = str(tmpdir)
Expand Down Expand Up @@ -896,13 +844,12 @@ def test_as_dict(tmpdir):

class TestBuildConfigV2(object):

def get_build_config(self, config, env_config=None,
source_file='readthedocs.yml', source_position=0):
def get_build_config(
self, config, env_config=None, source_file='readthedocs.yml'):
return BuildConfigV2(
env_config or {},
config,
source_file=source_file,
source_position=source_position,
)

def test_version(self):
Expand Down
Loading

0 comments on commit 1648006

Please sign in to comment.