Skip to content
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

Remove support for multiple configurations in one file #4800

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 16 additions & 40 deletions readthedocs/config/config.py
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 @@ -144,11 +141,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 @@ -160,10 +156,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 @@ -175,7 +168,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 @@ -189,7 +181,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 @@ -1043,16 +1034,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 @@ -1068,10 +1049,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 @@ -1080,23 +1060,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
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
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 @@ -131,10 +128,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 @@ -145,10 +139,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 @@ -159,10 +150,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 @@ -183,31 +171,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 @@ -217,7 +192,6 @@ def test_config_requires_name():
{'output_base': ''},
{},
source_file='readthedocs.yml',
source_position=0,
)
with raises(InvalidConfig) as excinfo:
build.validate()
Expand All @@ -230,7 +204,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 @@ -554,7 +527,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 @@ -576,7 +548,6 @@ def it_validates_to_abspath(tmpdir):
get_env_config(),
{'base': '../docs'},
source_file=source_file,
source_position=0,
)
build.validate()
assert build.base == str(tmpdir.join('docs'))
Expand All @@ -597,7 +568,6 @@ def it_fails_if_base_is_not_a_string(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 @@ -610,7 +580,6 @@ def it_fails_if_base_does_not_exist(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 @@ -626,7 +595,6 @@ def it_fails_if_build_is_invalid_option(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 @@ -642,7 +610,6 @@ def it_fails_on_python_validation(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 @@ -659,7 +626,6 @@ def it_works_on_python_validation(tmpdir):
'python': {'version': '3.3'},
},
source_file=str(tmpdir.join('readthedocs.yml')),
source_position=0,
)
build.validate_build()
build.validate_python()
Expand All @@ -670,7 +636,6 @@ def it_works(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 @@ -681,7 +646,6 @@ def default(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 @@ -697,7 +661,6 @@ def it_priorities_image_from_env_config(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 @@ -787,7 +750,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 @@ -803,20 +765,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 All @@ -843,13 +791,12 @@ def test_config_filenames_regex(correct_config_filename):

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