Skip to content

Commit

Permalink
Merge pull request #5510 from stsewd/show-syntax-errors-to-users
Browse files Browse the repository at this point in the history
Catch specific exception for config not found
  • Loading branch information
stsewd committed Mar 21, 2019
2 parents bb8b08f + 12f03e9 commit 4263c14
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 14 deletions.
21 changes: 16 additions & 5 deletions readthedocs/config/config.py
@@ -1,4 +1,3 @@
# -*- coding: utf-8 -*-
# pylint: disable=too-many-lines

"""Build configuration for rtd."""
Expand Down Expand Up @@ -37,13 +36,15 @@
validate_string,
)


__all__ = (
'ALL',
'load',
'BuildConfigV1',
'BuildConfigV2',
'ConfigError',
'ConfigOptionNotSupportedError',
'ConfigFileNotFound',
'InvalidConfig',
'PIP',
'SETUPTOOLS',
Expand All @@ -58,7 +59,7 @@
VERSION_INVALID = 'version-invalid'
CONFIG_SYNTAX_INVALID = 'config-syntax-invalid'
CONFIG_REQUIRED = 'config-required'
CONF_FILE_REQUIRED = 'conf-file-required'
CONFIG_FILE_REQUIRED = 'config-file-required'
PYTHON_INVALID = 'python-invalid'
SUBMODULES_INVALID = 'submodules-invalid'
INVALID_KEYS_COMBINATION = 'invalid-keys-combination'
Expand Down Expand Up @@ -87,6 +88,17 @@ def __init__(self, message, code):
super().__init__(message)


class ConfigFileNotFound(ConfigError):

"""Error when we can't find a configuration file."""

def __init__(self, directory):
super().__init__(
f"Configuration file not found in: {directory}",
CONFIG_FILE_REQUIRED,
)


class ConfigOptionNotSupportedError(ConfigError):

"""Error for unsupported configuration options in a version."""
Expand Down Expand Up @@ -298,7 +310,6 @@ class BuildConfigV1(BuildConfigBase):

"""Version 1 of the configuration file."""

CONF_FILE_REQUIRED_MESSAGE = 'Missing key "conf_file"'
PYTHON_INVALID_MESSAGE = '"python" section must be a mapping.'
PYTHON_EXTRA_REQUIREMENTS_INVALID_MESSAGE = (
'"python.extra_requirements" section must be a list.'
Expand Down Expand Up @@ -1104,14 +1115,14 @@ def load(path, env_config):
filename = find_one(path, CONFIG_FILENAME_REGEX)

if not filename:
raise ConfigError('No configuration file found', code=CONFIG_REQUIRED)
raise ConfigFileNotFound(path)
with open(filename, 'r') as configuration_file:
try:
config = parse(configuration_file.read())
except ParseError as error:
raise ConfigError(
'Parse error in {filename}: {message}'.format(
filename=filename,
filename=os.path.relpath(filename, path),
message=str(error),
),
code=CONFIG_SYNTAX_INVALID,
Expand Down
28 changes: 26 additions & 2 deletions readthedocs/config/tests/test_config.py
Expand Up @@ -15,14 +15,17 @@
BuildConfigV1,
BuildConfigV2,
ConfigError,
ConfigFileNotFound,
ConfigOptionNotSupportedError,
InvalidConfig,
load,
)
from readthedocs.config.config import (
CONFIG_FILE_REQUIRED,
CONFIG_FILENAME_REGEX,
CONFIG_NOT_SUPPORTED,
CONFIG_REQUIRED,
CONFIG_SYNTAX_INVALID,
INVALID_KEY,
PYTHON_INVALID,
VERSION_INVALID,
Expand Down Expand Up @@ -73,9 +76,9 @@ def get_build_config(config, env_config=None, source_file='readthedocs.yml'):
def test_load_no_config_file(tmpdir, files):
apply_fs(tmpdir, files)
base = str(tmpdir)
with raises(ConfigError) as e:
with raises(ConfigFileNotFound) as e:
load(base, {})
assert e.value.code == CONFIG_REQUIRED
assert e.value.code == CONFIG_FILE_REQUIRED


def test_load_empty_config_file(tmpdir):
Expand Down Expand Up @@ -136,6 +139,27 @@ def test_load_unknow_version(tmpdir):
assert excinfo.value.code == VERSION_INVALID


def test_load_raise_exception_invalid_syntax(tmpdir):
apply_fs(
tmpdir, {
'readthedocs.yml': textwrap.dedent('''
version: 2
python:
install:
- method: pip
path: .
# bad indentation here
extra_requirements:
- build
'''),
},
)
base = str(tmpdir)
with raises(ConfigError) as excinfo:
load(base, {})
assert excinfo.value.code == CONFIG_SYNTAX_INVALID


def test_yaml_extension(tmpdir):
"""Make sure loading the 'readthedocs' file with a 'yaml' extension."""
apply_fs(
Expand Down
11 changes: 4 additions & 7 deletions readthedocs/doc_builder/config.py
@@ -1,10 +1,8 @@
# -*- coding: utf-8 -*-

"""An API to load config from a readthedocs.yml file."""

from os import path

from readthedocs.config import BuildConfigV1, ConfigError, InvalidConfig
from readthedocs.config import BuildConfigV1, ConfigFileNotFound
from readthedocs.config import load as load_config
from readthedocs.projects.models import ProjectConfigurationError

Expand Down Expand Up @@ -59,10 +57,9 @@ def load_yaml_config(version):
path=checkout_path,
env_config=env_config,
)
except InvalidConfig:
# This is a subclass of ConfigError, so has to come first
raise
except ConfigError:
except ConfigFileNotFound:
# Dafault to use v1 with some defaults from the web interface
# if we don't find a configuration file.
config = BuildConfigV1(
env_config=env_config,
raw_config={},
Expand Down

0 comments on commit 4263c14

Please sign in to comment.