From cb3ea2d8e8a9b8a3d37706c696aff0f5e9112bc7 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Wed, 25 Dec 2024 16:54:43 -0500 Subject: [PATCH] Update handling of Replica creation (#7) This mirrors how we've done this in the C++ interface used by Taskwarrior: instead of trying to wrap `dyn Storage`, this just uses a few storage-specific methods to create replicas. --- README.md | 12 +++++++++++- src/lib.rs | 4 ---- src/replica.rs | 29 ++++++++++++++++------------- src/storage.rs | 38 -------------------------------------- taskchampion.pyi | 4 +++- tests/test_replica.py | 11 ++++------- tests/test_task.py | 20 ++++++++++---------- tests/test_working_set.py | 4 ++-- 8 files changed, 46 insertions(+), 76 deletions(-) delete mode 100644 src/storage.rs diff --git a/README.md b/README.md index cf09673..688edfe 100644 --- a/README.md +++ b/README.md @@ -8,6 +8,17 @@ It follows the TaskChampion API closely, with minimal adaptation for Python. The `taskchampion-py` package version matches the Rust crate's version. When an additional package release is required for the same Rust crate, a fourth version component is used; for example `1.2.0.1` for the second release of `taskchampion-py` containing TaskChampion version `1.2.0`. +## Usage + +```py +from taskchampion import Replica + +# Set up a replica. +r = Replica.new_on_disk("/some/path", true) + +# (more TBD) +``` + ## Development This project is built using [maturin](https://github.com/PyO3/maturin). @@ -45,5 +56,4 @@ poetry run pytest ## TODO -- There is no good way to describe functions that accept interface (e.g. `Replica::new` accepts any of the storage implementations, but Python bindings lack such mechanisms), currently, `Replica::new` just constructs the SqliteStorage from the params passed into the constructor. - Possible integration with Github Workflows for deployment to PyPI diff --git a/src/lib.rs b/src/lib.rs index 9e8c176..c2c6c3b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -3,8 +3,6 @@ pub mod replica; use replica::*; pub mod working_set; use working_set::*; -pub mod storage; -use storage::*; pub mod dependency_map; use dependency_map::*; pub mod operation; @@ -20,8 +18,6 @@ fn taskchampion(m: &Bound<'_, PyModule>) -> PyResult<()> { m.add_class::()?; m.add_class::()?; m.add_class::()?; - m.add_class::()?; - m.add_class::()?; m.add_class::()?; m.add_class::()?; diff --git a/src/replica.rs b/src/replica.rs index 8229e47..bd047d1 100644 --- a/src/replica.rs +++ b/src/replica.rs @@ -4,8 +4,7 @@ use std::rc::Rc; use crate::task::TaskData; use crate::{DependencyMap, Operation, Task, WorkingSet}; use pyo3::prelude::*; -use taskchampion::storage::{InMemoryStorage, SqliteStorage}; -use taskchampion::{Operations as TCOperations, Replica as TCReplica, Uuid}; +use taskchampion::{Operations as TCOperations, Replica as TCReplica, StorageConfig, Uuid}; #[pyclass] /// A replica represents an instance of a user's task data, providing an easy interface @@ -15,29 +14,33 @@ pub struct Replica(TCReplica); unsafe impl Send for Replica {} #[pymethods] impl Replica { - #[new] - /// Instantiates the Replica + #[staticmethod] + /// Create a Replica with on-disk storage. /// /// Args: /// path (str): path to the directory with the database /// create_if_missing (bool): create the database if it does not exist /// Raises: /// RuntimeError: if database does not exist, and create_if_missing is false - pub fn new(path: String, create_if_missing: bool) -> anyhow::Result { - let storage = SqliteStorage::new(path, create_if_missing)?; - - Ok(Replica(TCReplica::new(Box::new(storage)))) + pub fn new_on_disk(path: String, create_if_missing: bool) -> anyhow::Result { + Ok(Replica(TCReplica::new( + StorageConfig::OnDisk { + taskdb_dir: path.into(), + create_if_missing, + } + .into_storage()?, + ))) } #[staticmethod] - pub fn new_inmemory() -> Self { - let storage = InMemoryStorage::new(); - - Replica(TCReplica::new(Box::new(storage))) + pub fn new_in_memory() -> anyhow::Result { + Ok(Replica(TCReplica::new( + StorageConfig::InMemory.into_storage()?, + ))) } + /// Create a new task /// The task must not already exist. - pub fn create_task(&mut self, uuid: String) -> anyhow::Result<(Task, Vec)> { let mut ops = TCOperations::new(); let task = self diff --git a/src/storage.rs b/src/storage.rs deleted file mode 100644 index b7c1ae9..0000000 --- a/src/storage.rs +++ /dev/null @@ -1,38 +0,0 @@ -use pyo3::prelude::*; -use taskchampion::storage::{ - InMemoryStorage as TCInMemoryStorage, SqliteStorage as TCSqliteStorage, -}; -// TODO: actually make the storage usable and extensible, rn it just exists /shrug -pub trait Storage {} - -#[allow(dead_code)] -#[pyclass] -pub struct InMemoryStorage(TCInMemoryStorage); - -#[pymethods] -impl InMemoryStorage { - #[new] - pub fn new() -> InMemoryStorage { - InMemoryStorage(TCInMemoryStorage::new()) - } -} - -impl Storage for InMemoryStorage {} - -#[allow(dead_code)] -#[pyclass] -pub struct SqliteStorage(TCSqliteStorage); - -#[pymethods] -impl SqliteStorage { - #[new] - pub fn new(path: String, create_if_missing: bool) -> anyhow::Result { - // TODO: kinda ugly, prettify; - Ok(SqliteStorage(TCSqliteStorage::new( - path, - create_if_missing, - )?)) - } -} - -impl Storage for SqliteStorage {} diff --git a/taskchampion.pyi b/taskchampion.pyi index 4211dd4..21acd47 100644 --- a/taskchampion.pyi +++ b/taskchampion.pyi @@ -6,7 +6,9 @@ from typing import Optional, Iterator class Replica: def __init__(self, path: str, create_if_missing: bool): ... @staticmethod - def new_inmemory(): ... + def new_on_disk(path: str, create_if_missing: bool): ... + @staticmethod + def new_in_memory(): ... def create_task(self, uuid: str) -> tuple["Task", list["Operation"]]: ... def all_task_uuids(self) -> list[str]: ... def all_tasks(self) -> dict[str, "Task"]: ... diff --git a/tests/test_replica.py b/tests/test_replica.py index 45c9760..0d1f6d7 100644 --- a/tests/test_replica.py +++ b/tests/test_replica.py @@ -4,12 +4,9 @@ import pytest from taskchampion import Replica -# TODO: instantiate the in-memory replica, this will do for now - - @pytest.fixture -def empty_replica(tmp_path: Path) -> Replica: - return Replica(str(tmp_path), True) +def empty_replica() -> Replica: + return Replica.new_in_memory() @pytest.fixture @@ -30,14 +27,14 @@ def replica_with_tasks(empty_replica: Replica): def test_constructor(tmp_path: Path): - r = Replica(str(tmp_path), True) + r = Replica.new_on_disk(str(tmp_path), True) assert r is not None def test_constructor_throws_error_with_missing_database(tmp_path: Path): with pytest.raises(RuntimeError): - Replica(str(tmp_path), False) + Replica.new_on_disk(str(tmp_path), False) def test_create_task(empty_replica: Replica): diff --git a/tests/test_task.py b/tests/test_task.py index ac86b0c..db54054 100644 --- a/tests/test_task.py +++ b/tests/test_task.py @@ -5,8 +5,8 @@ @pytest.fixture -def new_task(tmp_path): - r = Replica(str(tmp_path), True) +def new_task(): + r = Replica.new_in_memory() result = r.create_task(str(uuid.uuid4())) assert result is not None @@ -16,8 +16,8 @@ def new_task(tmp_path): @pytest.fixture -def waiting_task(tmp_path): - r = Replica(str(tmp_path), True) +def waiting_task(): + r = Replica.new_in_memory() result = r.create_task(str(uuid.uuid4())) assert result is not None @@ -31,8 +31,8 @@ def waiting_task(tmp_path): @pytest.fixture -def started_task(tmp_path): - r = Replica(str(tmp_path), True) +def started_task(): + r = Replica.new_in_memory() result = r.create_task(str(uuid.uuid4())) assert result is not None @@ -43,8 +43,8 @@ def started_task(tmp_path): @pytest.fixture -def blocked_task(tmp_path): - r = Replica(str(tmp_path), True) +def blocked_task(): + r = Replica.new_in_memory() result = r.create_task(str(uuid.uuid4())) assert result is not None @@ -57,8 +57,8 @@ def blocked_task(tmp_path): @pytest.fixture -def due_task(tmp_path): - r = Replica(str(tmp_path), True) +def due_task(): + r = Replica.new_in_memory() task, _ = r.create_task(str(uuid.uuid4())) task.set_due(datetime.fromisoformat("2006-05-13T01:27:27+00:00")) diff --git a/tests/test_working_set.py b/tests/test_working_set.py index d5c76e5..b3b7c18 100644 --- a/tests/test_working_set.py +++ b/tests/test_working_set.py @@ -5,8 +5,8 @@ @pytest.fixture -def working_set(tmp_path: Path): - r = Replica(str(tmp_path), True) +def working_set(): + r = Replica.new_in_memory() ops = [] task, op = r.create_task(str(uuid.uuid4()))