Skip to content

Commit 57efd69

Browse files
committed
Patch core's configuration to allow get from global scope
...without requiring a dataset to be present. This patch replaces both frontend functions for `configuration`, because the input validation checks are not sufficiently modularized for a more surgical patch. Two key changes are made: 1) The unnecessary prevention of a `get` without a dataset around is removed, and the documentation is adjusted. 2) The `configuration()` function now selects the appropriate config manager based on the `ds` argument value alone, rather than implementing additional cross-comparison with the `scope` argument. Such argument validation is not done in `Configuration.__call__()` alone. Closes datalad/datalad#6854
1 parent 57b85fd commit 57efd69

File tree

4 files changed

+270
-0
lines changed

4 files changed

+270
-0
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<!-- Uncomment the section that is right (remove the HTML comment wrapper).-->
2+
<!--
3+
### 🐛 Bug Fixes
4+
5+
- Describe change, possibly reference closed/related issue/PR.
6+
Fixes [#XXX](https://github.com/datalad/datalad-next/issues/XXX)
7+
[#XXX](https://github.com/datalad/datalad-next/pull/XXX) (by @GITHUBHANDLE)
8+
-->
9+
### 💫 Enhancements and new features
10+
11+
- The `configuration` command no longer requires a datasets to be present
12+
for a `get` operation to retrieve a configuration item from scope `global`.
13+
Fixes [#6864](https://github.com/datalad/datalad/issues/6854) via
14+
[#86](https://github.com/datalad/datalad-next/pull/86) (by @mih)
15+
<!--
16+
### 🪓 Deprecations and removals
17+
18+
- Describe change, possibly reference closed/related issue/PR.
19+
Fixes [#XXX](https://github.com/datalad/datalad-next/issues/XXX)
20+
[#XXX](https://github.com/datalad/datalad-next/pull/XXX) (by @GITHUBHANDLE)
21+
-->
22+
<!--
23+
### 📝 Documentation
24+
25+
- Describe change, possibly reference closed/related issue/PR.
26+
Fixes [#XXX](https://github.com/datalad/datalad-next/issues/XXX)
27+
[#XXX](https://github.com/datalad/datalad-next/pull/XXX) (by @GITHUBHANDLE)
28+
-->
29+
<!--
30+
### 🏠 Internal
31+
32+
- Describe change, possibly reference closed/related issue/PR.
33+
Fixes [#XXX](https://github.com/datalad/datalad-next/issues/XXX)
34+
[#XXX](https://github.com/datalad/datalad-next/pull/XXX) (by @GITHUBHANDLE)
35+
-->
36+
<!--
37+
### 🛡 Tests
38+
39+
- Describe change, possibly reference closed/related issue/PR.
40+
Fixes [#XXX](https://github.com/datalad/datalad-next/issues/XXX)
41+
[#XXX](https://github.com/datalad/datalad-next/pull/XXX) (by @GITHUBHANDLE)
42+
-->

datalad_next/patches/__init__.py

+1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from . import (
22
annexrepo,
3+
configuration,
34
create_sibling_ghlike,
45
push_to_export_remote,
56
push_optimize,

datalad_next/patches/configuration.py

+199
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
1+
# emacs: -*- mode: python; py-indent-offset: 4; tab-width: 4; indent-tabs-mode: nil -*-
2+
# ex: set sts=4 ts=4 sw=4 noet:
3+
# ## ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ##
4+
#
5+
# See COPYING file distributed along with the datalad package for the
6+
# copyright and license terms.
7+
#
8+
# ## ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ##
9+
"""Frontend for the DataLad config"""
10+
11+
__docformat__ = 'restructuredtext'
12+
13+
14+
import logging
15+
16+
from datalad import cfg as dlcfg
17+
from datalad.distribution.dataset import (
18+
Dataset,
19+
datasetmethod,
20+
require_dataset,
21+
)
22+
from datalad.interface.base import (
23+
build_doc,
24+
)
25+
from datalad.interface.common_cfg import definitions as cfg_defs
26+
from datalad.interface.results import get_status_dict
27+
from datalad.interface.utils import (
28+
eval_results,
29+
)
30+
from datalad.local import configuration as conf_mod
31+
from datalad.local.configuration import (
32+
config_actions,
33+
_dump,
34+
_get,
35+
_set,
36+
_unset,
37+
)
38+
from datalad.support.exceptions import (
39+
NoDatasetFound,
40+
)
41+
from datalad.utils import (
42+
ensure_list,
43+
)
44+
45+
lgr = logging.getLogger('datalad.local.configuration')
46+
47+
48+
@build_doc
49+
class Configuration(conf_mod.Configuration):
50+
""""""
51+
@staticmethod
52+
@datasetmethod(name='configuration')
53+
@eval_results
54+
def __call__(
55+
action='dump',
56+
spec=None,
57+
*,
58+
scope=None,
59+
dataset=None,
60+
recursive=False,
61+
recursion_limit=None):
62+
63+
# check conditions
64+
# - global and recursion makes no sense
65+
66+
if action == 'dump':
67+
if scope:
68+
raise ValueError(
69+
'Scope selection is not supported for dumping')
70+
71+
# normalize variable specificatons
72+
specs = []
73+
for s in ensure_list(spec):
74+
if isinstance(s, tuple):
75+
specs.append((str(s[0]), str(s[1])))
76+
elif '=' not in s:
77+
specs.append((str(s),))
78+
else:
79+
specs.append(tuple(s.split('=', 1)))
80+
81+
if action == 'set':
82+
missing_values = [s[0] for s in specs if len(s) < 2]
83+
if missing_values:
84+
raise ValueError(
85+
'Values must be provided for all configuration '
86+
'settings. Missing: {}'.format(missing_values))
87+
invalid_names = [s[0] for s in specs if '.' not in s[0]]
88+
if invalid_names:
89+
raise ValueError(
90+
'Name must contain a section (i.e. "section.name"). '
91+
'Invalid: {}'.format(invalid_names))
92+
93+
ds = None
94+
if scope != 'global' or recursive:
95+
try:
96+
ds = require_dataset(
97+
dataset,
98+
check_installed=True,
99+
purpose='configure')
100+
except NoDatasetFound:
101+
if action not in ('dump', 'get') or dataset:
102+
raise
103+
104+
res_kwargs = dict(
105+
action='configuration',
106+
logger=lgr,
107+
)
108+
if ds:
109+
res_kwargs['refds'] = ds.path
110+
yield from configuration(action, scope, specs, res_kwargs, ds)
111+
112+
if not recursive:
113+
return
114+
115+
for subds in ds.subdatasets(
116+
state='present',
117+
recursive=True,
118+
recursion_limit=recursion_limit,
119+
on_failure='ignore',
120+
return_type='generator',
121+
result_renderer='disabled'):
122+
yield from configuration(
123+
action, scope, specs, res_kwargs, Dataset(subds['path']))
124+
125+
126+
def configuration(action, scope, specs, res_kwargs, ds=None):
127+
# go with the more specific dataset configmanager, if we are
128+
# operating on a dataset
129+
cfg = dlcfg if ds is None else ds.config
130+
131+
if action not in config_actions:
132+
raise ValueError("Unsupported action '{}'".format(action))
133+
134+
if action == 'dump':
135+
if not specs:
136+
# dumping is querying for all known keys
137+
specs = [
138+
(n,) for n in sorted(
139+
set(cfg_defs.keys()).union(cfg.keys()))
140+
]
141+
scope = None
142+
143+
for spec in specs:
144+
if '.' not in spec[0]:
145+
yield get_status_dict(
146+
ds=ds,
147+
status='error',
148+
message=(
149+
"Configuration key without a section: '%s'",
150+
spec[0],
151+
),
152+
**res_kwargs)
153+
continue
154+
# TODO without get-all there is little sense in having add
155+
#if action == 'add':
156+
# res = _add(cfg, scope, spec)
157+
if action == 'get':
158+
res = _get(cfg, scope, spec[0])
159+
elif action == 'dump':
160+
res = _dump(cfg, spec[0])
161+
# TODO this should be there, if we want to be comprehensive
162+
# however, we turned this off by default in the config manager
163+
# because we hardly use it, and the handling in ConfigManager
164+
# is not really well done.
165+
#elif action == 'get-all':
166+
# res = _get_all(cfg, scope, spec)
167+
elif action == 'set':
168+
res = _set(cfg, scope, *spec)
169+
elif action == 'unset':
170+
res = _unset(cfg, scope, spec[0])
171+
172+
if ds:
173+
res['path'] = ds.path
174+
175+
if 'status' not in res:
176+
res['status'] = 'ok'
177+
178+
yield dict(res_kwargs, **res)
179+
180+
if action in ('add', 'set', 'unset'):
181+
# we perform a single reload, rather than one for each modification
182+
# TODO: can we detect a call from cmdline? We could skip the reload.
183+
cfg.reload(force=True)
184+
185+
186+
conf_mod.Configuration.__call__ = Configuration.__call__
187+
conf_mod.Configuration._params_['scope']._doc = """\
188+
scope for getting or setting
189+
configuration. If no scope is declared for a query, all
190+
configuration sources (including overrides via environment
191+
variables) are considered according to the normal
192+
rules of precedence. A 'get' action can be constrained to
193+
scope 'branch', otherwise 'global' is used when not operating
194+
on a dataset, or 'local' (including 'global', when operating
195+
on a dataset.
196+
For action 'dump', a scope selection is ignored and all available
197+
scopes are considered."""
198+
conf_mod.Configuration.__call__.__doc__ = None
199+
conf_mod.Configuration = build_doc(conf_mod.Configuration)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
from datalad.tests.utils_pytest import (
2+
assert_in_results,
3+
chpwd,
4+
with_tempfile,
5+
)
6+
from datalad.api import (
7+
Dataset,
8+
configuration,
9+
)
10+
11+
# run all -core tests
12+
from datalad.local.tests.test_configuration import *
13+
14+
15+
@with_tempfile(mkdir=True)
16+
def test_config_get_global(path=None):
17+
"""Make sure `get` does not require a dataset to be present"""
18+
# enter a tempdir to be confident that there is no dataset around
19+
with chpwd(path):
20+
res = configuration('get', 'user.name', result_renderer='disabled')
21+
assert_in_results(
22+
res,
23+
name='user.name',
24+
status='ok',
25+
)
26+
# verify that the dataset method was replaced too
27+
ds = Dataset(path).create()
28+
assert "'get' action can be constrained" in ds.configuration.__doc__

0 commit comments

Comments
 (0)