Skip to content

cli/discover: add 'implicit' config to pair for collection creation #1177

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
4 changes: 4 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ Version 0.19.0
- Add a new ``showconfig`` status. This prints *some* configuration values as
JSON. This is intended to be used by external tools and helpers that interact
with ``vdirsyncer``, and considered experimental.
- Add ``implicit`` option to the :ref:`pair section <pair_config>`. When set to
"create", it implicitly creates missing collections during sync without user
prompts. This simplifies workflows where collections should be automatically
created on both sides.
- Update TLS-related tests that were failing due to weak MDs. :gh:`903`
- ``pytest-httpserver`` and ``trustme`` are now required for tests.
- ``pytest-localserver`` is no longer required for tests.
Expand Down
10 changes: 10 additions & 0 deletions docs/config.rst
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,16 @@ Pair Section

The ``conflict_resolution`` parameter applies for these properties too.

.. _implicit_def:

- ``implicit``: Opt into implicitly creating collections. Example::

implicit = "create"

When set to "create", missing collections are automatically created on both
sides during sync without prompting the user. This simplifies workflows where
all collections should be synchronized bidirectionally.

.. _storage_config:

Storage Section
Expand Down
59 changes: 59 additions & 0 deletions tests/system/cli/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,3 +222,62 @@ def test_validate_collections_param():
x([["c", None, "b"]])
x([["c", "a", None]])
x([["c", None, None]])


def test_invalid_implicit_value(read_config):
expected_message = "`implicit` parameter must be 'create' or absent"
with pytest.raises(exceptions.UserError) as excinfo:
read_config(
"""
[general]
status_path = "/tmp/status/"

[pair my_pair]
a = "my_a"
b = "my_b"
collections = null
implicit = "invalid"

[storage my_a]
type = "filesystem"
path = "{base}/path_a/"
fileext = ".txt"

[storage my_b]
type = "filesystem"
path = "{base}/path_b/"
fileext = ".txt"
"""
)

assert expected_message in str(excinfo.value)


def test_implicit_create_only(read_config):
"""Test that implicit create works."""
errors, c = read_config(
"""
[general]
status_path = "/tmp/status/"

[pair my_pair]
a = "my_a"
b = "my_b"
collections = ["from a", "from b"]
implicit = "create"

[storage my_a]
type = "filesystem"
path = "{base}/path_a/"
fileext = ".txt"

[storage my_b]
type = "filesystem"
path = "{base}/path_b/"
fileext = ".txt"
"""
)

assert not errors
pair = c.pairs["my_pair"]
assert pair.implicit == "create"
10 changes: 10 additions & 0 deletions vdirsyncer/cli/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,14 @@ def _validate_collections_param(collections):
raise ValueError(f"`collections` parameter, position {i}: {str(e)}")


def _validate_implicit_param(implicit):
if implicit is None:
return

if implicit != "create":
raise ValueError("`implicit` parameter must be 'create' or absent.")


class _ConfigReader:
def __init__(self, f: IO[Any]):
self._file: IO[Any] = f
Expand Down Expand Up @@ -229,6 +237,7 @@ def __init__(self, full_config: Config, name: str, options: dict[str, str]):
self.name: str = name
self.name_a: str = options.pop("a")
self.name_b: str = options.pop("b")
self.implicit = options.pop("implicit", None)

self._partial_sync: str | None = options.pop("partial_sync", None)
self.metadata = options.pop("metadata", None) or ()
Expand All @@ -247,6 +256,7 @@ def __init__(self, full_config: Config, name: str, options: dict[str, str]):
)
else:
_validate_collections_param(self.collections)
_validate_implicit_param(self.implicit)

if options:
raise ValueError("Unknown options: {}".format(", ".join(options)))
Expand Down
9 changes: 8 additions & 1 deletion vdirsyncer/cli/discover.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,13 @@ async def collections_for_pair(
connector=connector,
)

async def _handle_collection_not_found(
config, collection, e=None, implicit_create=False
):
return await handle_collection_not_found(
config, collection, e=e, implicit_create=pair.implicit == "create"
)

# We have to use a list here because the special None/null value would get
# mangled to string (because JSON objects always have string keys).
rv = await aiostream.stream.list(
Expand All @@ -102,7 +109,7 @@ async def collections_for_pair(
config_b=pair.config_b,
get_a_discovered=a_discovered.get_self,
get_b_discovered=b_discovered.get_self,
_handle_collection_not_found=handle_collection_not_found,
_handle_collection_not_found=_handle_collection_not_found,
)
)

Expand Down
8 changes: 5 additions & 3 deletions vdirsyncer/cli/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ async def storage_instance_from_config(
except exceptions.CollectionNotFound as e:
if create:
config = await handle_collection_not_found(
config, config.get("collection", None), e=str(e)
config, config.get("collection", None), e=str(e), implicit_create=True
)
return await storage_instance_from_config(
config,
Expand Down Expand Up @@ -342,7 +342,9 @@ def assert_permissions(path: str, wanted: int) -> None:
os.chmod(path, wanted)


async def handle_collection_not_found(config, collection, e=None):
async def handle_collection_not_found(
config, collection, e=None, implicit_create=False
):
storage_name = config.get("instance_name", None)

cli_logger.warning(
Expand All @@ -351,7 +353,7 @@ async def handle_collection_not_found(config, collection, e=None):
)
)

if click.confirm("Should vdirsyncer attempt to create it?"):
if implicit_create or click.confirm("Should vdirsyncer attempt to create it?"):
storage_type = config["type"]
cls, config = storage_class_from_config(config)
config["collection"] = collection
Expand Down