diff --git a/psycodict/config.py b/psycodict/config.py index d2a9c6a..7cda7f2 100644 --- a/psycodict/config.py +++ b/psycodict/config.py @@ -18,6 +18,52 @@ def strbool(s): raise ValueError(s) +def psycodict_home(): + """ + The directory used for psycodict's own files (the configuration file and + the slow-query log) when no explicit location is given: ``~/.psycodict``. + + Raises ``RuntimeError`` when no home directory can be determined -- for + example a container running with ``HOME`` unset and a uid that has no + passwd entry, where ``expanduser`` returns the literal ``~``. Refusing + loudly beats quietly creating a directory named ``~`` in the working + directory. + """ + home = os.path.expanduser("~") + if not os.path.isabs(home): + raise RuntimeError( + "Cannot determine a home directory for psycodict's configuration; " + "set the PSYCODICT_CONFIG environment variable (or pass " + "config_file) to choose a location for the configuration file" + ) + return os.path.join(home, ".psycodict") + + +def find_config_file(): + """ + The path of the configuration file when none is specified explicitly. + + This is the first of: + + - the ``PSYCODICT_CONFIG`` environment variable; + - ``config.ini`` in the current directory, if it exists; + - ``config.ini`` in ``~/.psycodict``. + + A missing configuration file is created at the resolved location. The + current-directory candidate requires the file to already exist, so + nothing is ever created in the working directory: a fresh setup gets its + configuration under ``~/.psycodict`` (or wherever ``PSYCODICT_CONFIG`` + points). When no home directory can be determined either, this raises + ``RuntimeError`` (see :func:`psycodict_home`). + """ + path = os.environ.get("PSYCODICT_CONFIG") + if path: + return os.path.abspath(os.path.expanduser(path)) + if os.path.exists("config.ini"): + return os.path.abspath("config.ini") + return os.path.join(psycodict_home(), "config.ini") + + class Configuration(): """ This configuration object merges input from the command line and a configuration file. @@ -30,21 +76,33 @@ class Configuration(): - ``parser`` -- an argparse.ArgumentParser instance. If not provided, a default will be created. - ``defaults`` -- a dictionary with default values for the created argument parser. Only used if a parser is not specified. The keys used are: - - ``config_file`` -- the filename for the configuration file + - ``config_file`` -- the filename for the configuration file. If not + given, it is discovered: the ``PSYCODICT_CONFIG`` environment + variable, then ``config.ini`` in the current directory if it exists, + then ``~/.psycodict/config.ini`` (created on first use); see + :func:`find_config_file`. + - ``secrets_file`` -- the filename for the secrets file, whose values + override the configuration file. If not given, ``secrets.ini`` next + to the configuration file. - ``logging_slowcutoff`` -- a float, giving the threshold above which a slow-query warning will be logged - - ``logging_slowlogfile`` -- a filename where slow-query warnings are printed + - ``logging_slowlogfile`` -- a filename where slow-query warnings are + printed. The default value ``slow_queries.log`` is placed in a + ``logs`` directory next to the configuration file, falling back to + ``~/.psycodict/logs`` when that location is not writable; any other + value is used verbatim. - ``postgresql_host`` -- the hostname for the database - ``postgresql_port`` -- an integer, the port to use when connecting to the database - ``postgresql_user`` -- the username when connecting to the database - ``postgresql_password`` -- the password for connecting to the database - - ``writeargstofile`` - a boolean, if config file doesn't exist, it determines if command line arguments are written to the config file instead of the default arguments - - ``readargs`` - a boolean, if determines if command line arguments are read + - ``postgresql_dbname`` -- the name of the database to connect to + - ``writeargstofile`` -- a boolean, if config file doesn't exist, it determines if command line arguments are written to the config file instead of the default arguments + - ``readargs`` -- a boolean (default False), determining whether command + line arguments are read. Leave this off in libraries and applications + with their own command line; pass True in a script whose command line + psycodict should parse. """ - def __init__(self, parser=None, defaults={}, writeargstofile=False, readargs=None): - if readargs is None: - import __main__ as main - # if a file was ran - readargs = hasattr(main, '__file__') + def __init__(self, parser=None, defaults={}, writeargstofile=False, readargs=False): + builtin_parser = parser is None if parser is None: parser = argparse.ArgumentParser(description="Default psycodict parser") @@ -54,16 +112,17 @@ def __init__(self, parser=None, defaults={}, writeargstofile=False, readargs=Non "--config-file", dest="config_file", metavar="FILE", - help="configuration file [default: %(default)s]", - default=defaults.get("config_file", "config.ini"), + help="configuration file [default: $PSYCODICT_CONFIG, " + "./config.ini if present, else ~/.psycodict/config.ini]", + default=defaults.get("config_file"), ) parser.add_argument( "-s", "--secrets-file", dest="secrets_file", metavar="SECRETS", - help="secrets file [default: %(default)s]", - default=defaults.get("secrets_file", "secrets.ini"), + help="secrets file [default: secrets.ini next to the configuration file]", + default=defaults.get("secrets_file"), ) logginggroup = parser.add_argument_group("Logging options:") @@ -123,7 +182,7 @@ def __init__(self, parser=None, defaults={}, writeargstofile=False, readargs=Non dest="postgresql_dbname", metavar="DBNAME", help="PostgreSQL database name [default: %(default)s]", - default="lmfdb", + default=defaults.get("postgresql_dbname", "lmfdb"), ) def sec_opt(key): @@ -141,6 +200,19 @@ def sec_opt(key): # only read config file args = parser.parse_args([]) + # Resolve the file locations. An explicit location (from the + # defaults dictionary or the command line) is used verbatim; with the + # builtin parser both default to None, triggering the discovery in + # find_config_file -- so a missing configuration file is created + # under ~/.psycodict (or $PSYCODICT_CONFIG), never in the working + # directory. + if args.config_file is None: + args.config_file = find_config_file() + if args.secrets_file is None: + args.secrets_file = os.path.join( + os.path.dirname(os.path.abspath(args.config_file)), "secrets.ini" + ) + args_dict = vars(args) default_arguments_dict = vars(parser.parse_args([])) @@ -179,6 +251,11 @@ def sec_opt(key): for opt, val in options.items(): _cfgp.set(sec, opt, str(val)) + # the resolved location may sit in a directory that does not + # exist yet (a fresh ~/.psycodict in particular) + confdir = os.path.dirname(args.config_file) + if confdir: + os.makedirs(confdir, exist_ok=True) with open(args.config_file, "w") as configfile: _cfgp.write(configfile) @@ -238,6 +315,50 @@ def get(section, key): if key not in default_arguments_dict: self.extra_options[key] = val + if builtin_parser: + # The default value of slowlogfile used to be created in the + # working directory; place it in a logs directory next to the + # configuration file instead, falling back to ~/.psycodict/logs + # when that location cannot be written (a system-managed + # configuration in a read-only directory, say -- reading a + # configuration must not require write access beside it). Any + # other value is used verbatim. (Callers supplying their own + # parser own their logging semantics; they are not touched.) + logopts = self.options["logging"] + if logopts.get("slowlogfile") == "slow_queries.log": + logdir = self._default_log_dir(args.config_file) + if logdir is not None: + logopts["slowlogfile"] = os.path.join(logdir, "slow_queries.log") + + @staticmethod + def _default_log_dir(config_file): + """ + The directory for the default slow-query log: the first of + ``/logs`` and ``~/.psycodict/logs`` that exists or can be + created, and is writable. + + The directory must exist and be writable before PostgresDatabase + attaches its FileHandler, so the candidates are probed by creating + them. Returns None when neither candidate is usable (the plain + default name is then kept, preserving the historical behavior of + writing in the working directory). + """ + candidates = [ + os.path.join(os.path.dirname(os.path.abspath(config_file)), "logs") + ] + try: + candidates.append(os.path.join(psycodict_home(), "logs")) + except RuntimeError: + pass + for candidate in candidates: + try: + os.makedirs(candidate, exist_ok=True) + except OSError: + continue + if os.access(candidate, os.W_OK | os.X_OK): + return candidate + return None + def get_postgresql_default(self): res = dict(self.default_args["postgresql"]) res["port"] = int(res["port"]) diff --git a/tests/test_config.py b/tests/test_config.py index 8f1eef9..b039c02 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -284,15 +284,17 @@ def test_command_line_values_beat_the_config_file(paths, monkeypatch): assert config.options["postgresql"]["user"] == "fileuser" -@pytest.mark.parametrize("has_file,expected", [(True, "cli.example.com"), (False, "localhost")]) -def test_readargs_defaults_to_whether_main_is_a_script(paths, monkeypatch, has_file, expected): +def test_readargs_defaults_to_false_even_in_a_script(paths, monkeypatch): + # The command line belongs to the host program: psycodict must not parse + # it unless asked. It used to auto-detect "running as a script" (via + # __main__.__file__) and then argparse would reject the host's own + # options with a SystemExit; pin that both are gone. main = types.ModuleType("fake_main") - if has_file: - main.__file__ = "script.py" + main.__file__ = "script.py" monkeypatch.setitem(sys.modules, "__main__", main) - monkeypatch.setattr(sys, "argv", ["prog", "--postgresql-host", "cli.example.com"]) + monkeypatch.setattr(sys, "argv", ["prog", "--no-such-option", "--postgresql-host", "cli.example.com"]) config = Configuration(defaults=paths) - assert config.options["postgresql"]["host"] == expected + assert config.options["postgresql"]["host"] == "localhost" def test_extra_options_holds_the_arguments_not_stored_in_the_file(paths): @@ -303,6 +305,213 @@ def test_extra_options_holds_the_arguments_not_stored_in_the_file(paths): } +# --------------------------------------------------------------------------- +# file discovery +# --------------------------------------------------------------------------- + + +@pytest.fixture +def isolated(tmp_path, monkeypatch): + """ + An isolated environment for the discovery tests: an empty working + directory, a throwaway HOME, and no PSYCODICT_CONFIG. + + Returns the fake home directory (so ``~/.psycodict`` resolves under it). + """ + cwd = tmp_path / "cwd" + home = tmp_path / "home" + cwd.mkdir() + home.mkdir() + monkeypatch.chdir(cwd) + monkeypatch.setenv("HOME", str(home)) + monkeypatch.delenv("PSYCODICT_CONFIG", raising=False) + return home + + +def test_discovery_falls_back_to_the_psycodict_home(isolated): + config = Configuration(readargs=False) + created = isolated / ".psycodict" / "config.ini" + assert created.exists() + assert config.extra_options["config_file"] == str(created) + assert config.options["postgresql"]["host"] == "localhost" + # ... and nothing was created in the working directory + assert not os.path.exists("config.ini") + + +def test_discovery_uses_an_existing_cwd_config(isolated): + write_ini("config.ini", {"postgresql": {"host": "cwdhost"}}) + config = Configuration(readargs=False) + assert config.options["postgresql"]["host"] == "cwdhost" + assert config.extra_options["config_file"] == os.path.abspath("config.ini") + # the fallback location is not touched + assert not (isolated / ".psycodict").exists() + + +def test_discovery_env_var_beats_the_cwd_config(isolated, tmp_path, monkeypatch): + write_ini("config.ini", {"postgresql": {"host": "cwdhost"}}) + target = tmp_path / "elsewhere" / "psycodict.ini" + monkeypatch.setenv("PSYCODICT_CONFIG", str(target)) + config = Configuration(readargs=False) + # created at the environment location, with the defaults + assert target.exists() + assert config.extra_options["config_file"] == str(target) + assert config.options["postgresql"]["host"] == "localhost" + + +def test_an_explicit_config_file_beats_the_environment(isolated, tmp_path, monkeypatch, paths): + monkeypatch.setenv("PSYCODICT_CONFIG", str(tmp_path / "env.ini")) + config = Configuration(defaults=paths, readargs=False) + assert config.extra_options["config_file"] == paths["config_file"] + assert not (tmp_path / "env.ini").exists() + + +def test_the_default_secrets_file_sits_next_to_the_config_file(isolated): + confdir = isolated / ".psycodict" + confdir.mkdir() + write_ini(str(confdir / "secrets.ini"), {"postgresql": {"password": "s3cret"}}) + config = Configuration(readargs=False) + assert config.extra_options["secrets_file"] == str(confdir / "secrets.ini") + assert config.options["postgresql"]["password"] == "s3cret" + + +# --------------------------------------------------------------------------- +# the slow-query log location +# --------------------------------------------------------------------------- + + +def test_the_default_slowlogfile_lands_in_logs_next_to_the_config(isolated): + config = Configuration(readargs=False) + logdir = isolated / ".psycodict" / "logs" + assert config.options["logging"]["slowlogfile"] == str(logdir / "slow_queries.log") + # PostgresDatabase attaches a FileHandler immediately, so the directory + # must already exist + assert logdir.is_dir() + # the file itself records the default name; the redirect is applied when + # the configuration is read, so editing the file still works + written = read_ini(str(isolated / ".psycodict" / "config.ini")) + assert written.get("logging", "slowlogfile") == "slow_queries.log" + + +def test_an_explicit_slowlogfile_is_respected(isolated, tmp_path): + target = str(tmp_path / "my_slow.log") + config = Configuration( + defaults={"logging_slowlogfile": target}, readargs=False + ) + assert config.options["logging"]["slowlogfile"] == target + + +def test_a_slowlogfile_from_the_config_file_is_respected(isolated, tmp_path): + target = str(tmp_path / "configured.log") + write_ini("config.ini", {"logging": {"slowlogfile": target}}) + config = Configuration(readargs=False) + assert config.options["logging"]["slowlogfile"] == target + assert not os.path.exists("logs") + + +def test_the_default_name_in_a_config_file_is_also_redirected(isolated): + # An existing file that kept the default name gets the same treatment as + # a fresh one: the name is treated as "unconfigured", not as a request + # for a file of that name in the working directory. + write_ini("config.ini", {"logging": {"slowlogfile": "slow_queries.log"}}) + config = Configuration(readargs=False) + expected = os.path.join(os.path.abspath("logs"), "slow_queries.log") + assert config.options["logging"]["slowlogfile"] == expected + + +def test_a_supplied_parser_gets_no_log_redirect(isolated, paths): + # Callers with their own parser own their logging semantics. + parser = make_parser(paths) + parser.add_argument("--slowlogfile", dest="logging_slowlogfile", + default="slow_queries.log") + config = Configuration(parser=parser, readargs=False) + assert config.options["logging"]["slowlogfile"] == "slow_queries.log" + + +needs_permissions = pytest.mark.skipif( + hasattr(os, "geteuid") and os.geteuid() == 0, + reason="root ignores directory permissions", +) + + +@pytest.fixture +def read_only_config(isolated, tmp_path, monkeypatch): + """ + A readable configuration file in a directory without write access, the + shape of a system-managed /etc/psycodict/config.ini. Restores the + permissions afterward so tmp_path can be cleaned up. + """ + confdir = tmp_path / "etc" + confdir.mkdir() + write_ini(str(confdir / "config.ini"), {"postgresql": {"host": "etchost"}}) + confdir.chmod(0o555) + monkeypatch.setenv("PSYCODICT_CONFIG", str(confdir / "config.ini")) + yield confdir + confdir.chmod(0o755) + + +@needs_permissions +def test_a_config_in_a_read_only_directory_is_readable(isolated, read_only_config): + # Reading a configuration must not require write access beside it; the + # default slow log falls back to ~/.psycodict/logs. + config = Configuration(readargs=False) + assert config.options["postgresql"]["host"] == "etchost" + expected = os.path.join(str(isolated), ".psycodict", "logs", "slow_queries.log") + assert config.options["logging"]["slowlogfile"] == expected + assert os.path.isdir(os.path.dirname(expected)) + + +@needs_permissions +def test_no_usable_log_directory_keeps_the_plain_name(isolated, read_only_config, monkeypatch): + # With the config directory read-only and no home either, the redirect + # gives up and the historical cwd-relative default survives. + monkeypatch.setattr(os.path, "expanduser", lambda path: path) + config = Configuration(readargs=False) + assert config.options["logging"]["slowlogfile"] == "slow_queries.log" + + +# --------------------------------------------------------------------------- +# missing home directory +# --------------------------------------------------------------------------- + + +def test_no_home_directory_is_a_clear_error_not_a_tilde_directory(isolated, monkeypatch): + # In a container with HOME unset and a uid without a passwd entry, + # expanduser returns the literal ~. The home fallback must refuse + # loudly instead of creating ./~/.psycodict in the working directory. + monkeypatch.setattr(os.path, "expanduser", lambda path: path) + with pytest.raises(RuntimeError, match="PSYCODICT_CONFIG"): + Configuration(readargs=False) + assert not os.path.exists("~") + + +def test_a_cwd_config_needs_no_home_directory(isolated, monkeypatch): + # The error is confined to the home fallback: with a discoverable + # configuration the homeless container works fine (and the log lands + # next to the config, not in a literal ~). + write_ini("config.ini", {"postgresql": {"host": "cwdhost"}}) + monkeypatch.setattr(os.path, "expanduser", lambda path: path) + config = Configuration(readargs=False) + assert config.options["postgresql"]["host"] == "cwdhost" + expected = os.path.join(os.path.abspath("logs"), "slow_queries.log") + assert config.options["logging"]["slowlogfile"] == expected + assert not os.path.exists("~") + + +# --------------------------------------------------------------------------- +# postgresql_dbname +# --------------------------------------------------------------------------- + + +def test_postgresql_dbname_default_is_honoured(paths): + # dbname used to be the one option whose default ignored the defaults + # dictionary. + config = Configuration( + defaults=dict(paths, postgresql_dbname="mydb"), readargs=False + ) + assert config.options["postgresql"]["dbname"] == "mydb" + assert read_ini(paths["config_file"]).get("postgresql", "dbname") == "mydb" + + # --------------------------------------------------------------------------- # a supplied parser # ---------------------------------------------------------------------------