forked from github/pelican
Merge pull request #2940 from r4victor/fix-issue-2938-attempt-2
Fix parsing of settings passed to `--extra-settings`
This commit is contained in:
commit
21c331e789
6 changed files with 114 additions and 60 deletions
3
RELEASE.md
Normal file
3
RELEASE.md
Normal file
|
|
@ -0,0 +1,3 @@
|
|||
Release type: patch
|
||||
|
||||
Fix incorrect parsing of parameters specified via `-e` / `--extra-settings` option flags (#2938).
|
||||
|
|
@ -9,11 +9,12 @@ line::
|
|||
If you used the ``pelican-quickstart`` command, your primary settings file will
|
||||
be named ``pelicanconf.py`` by default.
|
||||
|
||||
You can also specify extra settings via ``-e`` / ``--extra-settings`` option
|
||||
flags, which will override default settings as well as any defined within
|
||||
settings files::
|
||||
You can also specify settings via ``-e`` / ``--extra-settings`` option
|
||||
flags. It will override default settings as well as any defined within the
|
||||
setting file. Note that values must follow JSON notation::
|
||||
|
||||
pelican content -e SITENAME='"A site"' READERS='{"html": null}' CACHE_CONTENT=true
|
||||
|
||||
pelican content -e DELETE_OUTPUT_DIRECTORY=true
|
||||
|
||||
.. note::
|
||||
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import argparse
|
||||
import json
|
||||
import logging
|
||||
import multiprocessing
|
||||
import os
|
||||
|
|
@ -24,7 +25,7 @@ from pelican.plugins import signals
|
|||
from pelican.plugins._utils import get_plugin_name, load_plugins
|
||||
from pelican.readers import Readers
|
||||
from pelican.server import ComplexHTTPRequestHandler, RootedHTTPServer
|
||||
from pelican.settings import coerce_overrides, read_settings
|
||||
from pelican.settings import read_settings
|
||||
from pelican.utils import (FileSystemWatcher, clean_output_dir, maybe_pluralize)
|
||||
from pelican.writers import Writer
|
||||
|
||||
|
|
@ -259,16 +260,29 @@ class PrintSettings(argparse.Action):
|
|||
parser.exit()
|
||||
|
||||
|
||||
class ParseDict(argparse.Action):
|
||||
class ParseOverrides(argparse.Action):
|
||||
def __call__(self, parser, namespace, values, option_string=None):
|
||||
d = {}
|
||||
if values:
|
||||
for item in values:
|
||||
split_items = item.split("=", 1)
|
||||
key = split_items[0].strip()
|
||||
value = split_items[1].strip()
|
||||
d[key] = value
|
||||
setattr(namespace, self.dest, d)
|
||||
overrides = {}
|
||||
for item in values:
|
||||
try:
|
||||
k, v = item.split("=", 1)
|
||||
except ValueError:
|
||||
raise ValueError(
|
||||
'Extra settings must be specified as KEY=VALUE pairs '
|
||||
f'but you specified {item}'
|
||||
)
|
||||
try:
|
||||
overrides[k] = json.loads(v)
|
||||
except json.decoder.JSONDecodeError:
|
||||
raise ValueError(
|
||||
f'Invalid JSON value: {v}. '
|
||||
'Values specified via -e / --extra-settings flags '
|
||||
'must be in JSON notation. '
|
||||
'Use -e KEY=\'"string"\' to specify a string value; '
|
||||
'-e KEY=null to specify None; '
|
||||
'-e KEY=false (or true) to specify False (or True).'
|
||||
)
|
||||
setattr(namespace, self.dest, overrides)
|
||||
|
||||
|
||||
def parse_arguments(argv=None):
|
||||
|
|
@ -366,13 +380,13 @@ def parse_arguments(argv=None):
|
|||
|
||||
parser.add_argument('-e', '--extra-settings', dest='overrides',
|
||||
help='Specify one or more SETTING=VALUE pairs to '
|
||||
'override settings. If VALUE contains spaces, '
|
||||
'add quotes: SETTING="VALUE". Values other than '
|
||||
'integers and strings can be specified via JSON '
|
||||
'notation. (e.g., SETTING=none)',
|
||||
'override settings. VALUE must be in JSON notation: '
|
||||
'specify string values as SETTING=\'"some string"\'; '
|
||||
'booleans as SETTING=true or SETTING=false; '
|
||||
'None as SETTING=null.',
|
||||
nargs='*',
|
||||
action=ParseDict
|
||||
)
|
||||
action=ParseOverrides,
|
||||
default={})
|
||||
|
||||
args = parser.parse_args(argv)
|
||||
|
||||
|
|
@ -385,6 +399,8 @@ def parse_arguments(argv=None):
|
|||
|
||||
|
||||
def get_config(args):
|
||||
"""Builds a config dictionary based on supplied `args`.
|
||||
"""
|
||||
config = {}
|
||||
if args.path:
|
||||
config['PATH'] = os.path.abspath(os.path.expanduser(args.path))
|
||||
|
|
@ -409,7 +425,7 @@ def get_config(args):
|
|||
if args.bind is not None:
|
||||
config['BIND'] = args.bind
|
||||
config['DEBUG'] = args.verbosity == logging.DEBUG
|
||||
config.update(coerce_overrides(args.overrides))
|
||||
config.update(args.overrides)
|
||||
|
||||
return config
|
||||
|
||||
|
|
|
|||
|
|
@ -1,7 +1,6 @@
|
|||
import copy
|
||||
import importlib.util
|
||||
import inspect
|
||||
import json
|
||||
import locale
|
||||
import logging
|
||||
import os
|
||||
|
|
@ -659,25 +658,3 @@ def configure_settings(settings):
|
|||
continue # setting not specified, nothing to do
|
||||
|
||||
return settings
|
||||
|
||||
|
||||
def coerce_overrides(overrides):
|
||||
if overrides is None:
|
||||
return {}
|
||||
coerced = {}
|
||||
types_to_cast = {int, str, bool}
|
||||
for k, v in overrides.items():
|
||||
if k not in DEFAULT_CONFIG:
|
||||
logger.warning('Override for unknown setting %s, ignoring', k)
|
||||
continue
|
||||
setting_type = type(DEFAULT_CONFIG[k])
|
||||
if setting_type not in types_to_cast:
|
||||
coerced[k] = json.loads(v)
|
||||
else:
|
||||
try:
|
||||
coerced[k] = setting_type(v)
|
||||
except ValueError:
|
||||
logger.debug('ValueError for %s override with %s, try to '
|
||||
'load as json', k, v)
|
||||
coerced[k] = json.loads(v)
|
||||
return coerced
|
||||
|
|
|
|||
72
pelican/tests/test_cli.py
Normal file
72
pelican/tests/test_cli.py
Normal file
|
|
@ -0,0 +1,72 @@
|
|||
import unittest
|
||||
|
||||
from pelican import get_config, parse_arguments
|
||||
|
||||
|
||||
class TestParseOverrides(unittest.TestCase):
|
||||
def test_flags(self):
|
||||
for flag in ['-e', '--extra-settings']:
|
||||
args = parse_arguments([flag, 'k=1'])
|
||||
self.assertDictEqual(args.overrides, {'k': 1})
|
||||
|
||||
def test_parse_multiple_items(self):
|
||||
args = parse_arguments('-e k1=1 k2=2'.split())
|
||||
self.assertDictEqual(args.overrides, {'k1': 1, 'k2': 2})
|
||||
|
||||
def test_parse_valid_json(self):
|
||||
json_values_python_values_map = {
|
||||
'""': '',
|
||||
'null': None,
|
||||
'"string"': 'string',
|
||||
'["foo", 12, "4", {}]': ['foo', 12, '4', {}]
|
||||
}
|
||||
for k, v in json_values_python_values_map.items():
|
||||
args = parse_arguments(['-e', 'k=' + k])
|
||||
self.assertDictEqual(args.overrides, {'k': v})
|
||||
|
||||
def test_parse_invalid_syntax(self):
|
||||
invalid_items = ['k= 1', 'k =1', 'k', 'k v']
|
||||
for item in invalid_items:
|
||||
with self.assertRaises(ValueError):
|
||||
parse_arguments(f'-e {item}'.split())
|
||||
|
||||
def test_parse_invalid_json(self):
|
||||
invalid_json = {
|
||||
'', 'False', 'True', 'None', 'some other string',
|
||||
'{"foo": bar}', '[foo]'
|
||||
}
|
||||
for v in invalid_json:
|
||||
with self.assertRaises(ValueError):
|
||||
parse_arguments(['-e ', 'k=' + v])
|
||||
|
||||
|
||||
class TestGetConfigFromArgs(unittest.TestCase):
|
||||
def test_overrides_known_keys(self):
|
||||
args = parse_arguments([
|
||||
'-e',
|
||||
'DELETE_OUTPUT_DIRECTORY=false',
|
||||
'OUTPUT_RETENTION=["1.txt"]',
|
||||
'SITENAME="Title"'
|
||||
])
|
||||
config = get_config(args)
|
||||
config_must_contain = {
|
||||
'DELETE_OUTPUT_DIRECTORY': False,
|
||||
'OUTPUT_RETENTION': ['1.txt'],
|
||||
'SITENAME': 'Title'
|
||||
}
|
||||
self.assertDictEqual(config, {**config, **config_must_contain})
|
||||
|
||||
def test_overrides_non_default_type(self):
|
||||
args = parse_arguments([
|
||||
'-e',
|
||||
'DISPLAY_PAGES_ON_MENU=123',
|
||||
'PAGE_TRANSLATION_ID=null',
|
||||
'TRANSLATION_FEED_RSS_URL="someurl"'
|
||||
])
|
||||
config = get_config(args)
|
||||
config_must_contain = {
|
||||
'DISPLAY_PAGES_ON_MENU': 123,
|
||||
'PAGE_TRANSLATION_ID': None,
|
||||
'TRANSLATION_FEED_RSS_URL': 'someurl'
|
||||
}
|
||||
self.assertDictEqual(config, {**config, **config_must_contain})
|
||||
|
|
@ -7,7 +7,7 @@ from sys import platform
|
|||
|
||||
from pelican.settings import (DEFAULT_CONFIG, DEFAULT_THEME,
|
||||
_printf_s_to_format_field,
|
||||
coerce_overrides, configure_settings,
|
||||
configure_settings,
|
||||
handle_deprecated_settings, read_settings)
|
||||
from pelican.tests.support import unittest
|
||||
|
||||
|
|
@ -304,18 +304,3 @@ class TestSettingsConfiguration(unittest.TestCase):
|
|||
[(r'C\+\+', 'cpp')] +
|
||||
self.settings['SLUG_REGEX_SUBSTITUTIONS'])
|
||||
self.assertNotIn('SLUG_SUBSTITUTIONS', settings)
|
||||
|
||||
def test_coerce_overrides(self):
|
||||
overrides = coerce_overrides({
|
||||
'ARTICLE_EXCLUDES': '["testexcl"]',
|
||||
'READERS': '{"foo": "bar"}',
|
||||
'STATIC_EXCLUDE_SOURCES': 'true',
|
||||
'THEME_STATIC_DIR': 'theme',
|
||||
})
|
||||
expected = {
|
||||
'ARTICLE_EXCLUDES': ["testexcl"],
|
||||
'READERS': {"foo": "bar"},
|
||||
'STATIC_EXCLUDE_SOURCES': True,
|
||||
'THEME_STATIC_DIR': 'theme',
|
||||
}
|
||||
self.assertDictEqual(overrides, expected)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue