Skip to content

Store the progress & diff base in file and sqlite storages [v2]#1262

Draft
nolar wants to merge 9 commits into
mainfrom
more-storages
Draft

Store the progress & diff base in file and sqlite storages [v2]#1262
nolar wants to merge 9 commits into
mainfrom
more-storages

Conversation

@nolar
Copy link
Copy Markdown
Owner

@nolar nolar commented Mar 2, 2026 •
edited
Loading

While storing Kopf's state in the resource's annotations is the best & easiest way, some people might be unhappy about many API calls and heavy payloads. There was always a possibility to implement custom strages, though not documented well — now documented.

As an example of custom storages, Kopf now bundles with a couple of "out of the box" storages, specifically the filesystem & sqlite storages. However, they require configuring a shared volume, so they cannot work "out of the box" as a default storage — but they can be a good starting point for custom storages for users.

No external dependencies are needed, StdLib is enough (however, sqlite3 can be broken or not compiled, so imported only on demand).

Supersedes the AI-generated PR (too many micro-issues there, which I had to rewrite by hand, massively):

TODOs:

  • File storage: atomic file writes (with renames, or with file locks).
  • Sqlite storage: shared connection management via context manager
  • Sqlite storage: import sqlite3 only on demand, broken sometimes
  • All storages must be context managers, too (globally entered/exited)
  • All storages must be async (fetch/store/purge/erase methods at least).
    • 🔥 But backwards compatible with sync overrides.
      • BEWARE the super().fetch(…) trap - not awaited in descendants.
    • With a DeprecationWarning on the legacy sync override (via __new__? or in the post-startup checks?)
  • There should be only one storage for all data types, progress and diffbase, with a shared connection (pool).
    • But backwards compatible with separate storages if used so.
    • With a DeprecationWarning on the legacy separate mode usage.
    • Alternatively: accept a pre-created shared connection/pool to the constructor. This seems easier for transition.
  • Tests for all this.

@nolar nolar added the enhancement New feature or request label Mar 2, 2026
@nolar nolar mentioned this pull request Mar 2, 2026
Closed
github-advanced-security[bot]
github-advanced-security AI found potential problems Mar 2, 2026
View reviewed changes
Comment thread tests/persistence/test_storing_of_diffbase_in_files.py Outdated
@@ -0,0 +1,283 @@
import pytest

Check notice

Code scanning / CodeQL

Unused import

Import of 'pytest' is not used.

Copilot Autofix

AI 2 months ago

In general, unused import problems are best fixed by removing the import statements for modules that are not referenced in the file. This keeps dependencies minimal, improves readability, and avoids confusion about which libraries are actually required.

For this specific case in tests/persistence/test_storing_of_diffbase_in_files.py, the single best fix is to delete the import pytest line at the top of the file and leave the rest of the code unchanged. This does not change any existing functionality because test discovery and execution by pytest do not require importing the pytest module unless its APIs are explicitly used. Concretely, remove line 1 (import pytest) and keep the import yaml and subsequent imports as they are.

No additional methods, imports, or definitions are needed to implement this change.

Suggested changeset 1
tests/persistence/test_storing_of_diffbase_in_files.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/tests/persistence/test_storing_of_diffbase_in_files.py b/tests/persistence/test_storing_of_diffbase_in_files.py
--- a/tests/persistence/test_storing_of_diffbase_in_files.py
+++ b/tests/persistence/test_storing_of_diffbase_in_files.py
@@ -1,4 +1,3 @@
-import pytest
 import yaml
 
 from kopf._cogs.configs.diffbase import FileDiffBaseStorage
EOF
@@ -1,4 +1,3 @@
import pytest
import yaml

from kopf._cogs.configs.diffbase import FileDiffBaseStorage
Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
Comment thread tests/persistence/test_storing_of_progress_in_files.py Outdated
import yaml

from kopf._cogs.configs.progress import FileProgressStorage, ProgressRecord
from kopf._cogs.structs.bodies import Body, BodyEssence

Check notice

Code scanning / CodeQL

Unused import

Import of 'BodyEssence' is not used.

Copilot Autofix

AI 2 months ago

To fix an unused import, either remove the unused name from the import statement or start using it meaningfully. Since the tests clearly only need Body, the simplest and safest change is to stop importing BodyEssence.

Concretely, in tests/persistence/test_storing_of_progress_in_files.py, adjust the import on line 5 so that it only imports Body from kopf._cogs.structs.bodies. No other code changes are required, as there are no usages of BodyEssence in the provided snippet.

Suggested changeset 1
tests/persistence/test_storing_of_progress_in_files.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/tests/persistence/test_storing_of_progress_in_files.py b/tests/persistence/test_storing_of_progress_in_files.py
--- a/tests/persistence/test_storing_of_progress_in_files.py
+++ b/tests/persistence/test_storing_of_progress_in_files.py
@@ -2,7 +2,7 @@
 import yaml
 
 from kopf._cogs.configs.progress import FileProgressStorage, ProgressRecord
-from kopf._cogs.structs.bodies import Body, BodyEssence
+from kopf._cogs.structs.bodies import Body
 from kopf._cogs.structs.ids import HandlerId
 from kopf._cogs.structs.patches import Patch
 
EOF
@@ -2,7 +2,7 @@
import yaml

from kopf._cogs.configs.progress import FileProgressStorage, ProgressRecord
from kopf._cogs.structs.bodies import Body, BodyEssence
from kopf._cogs.structs.bodies import Body
from kopf._cogs.structs.ids import HandlerId
from kopf._cogs.structs.patches import Patch

Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
Comment thread tests/persistence/test_storing_of_progress_in_sqlite.py Outdated
@@ -0,0 +1,339 @@
import json

Check notice

Code scanning / CodeQL

Unused import

Import of 'json' is not used.

Copilot Autofix

AI 2 months ago

To fix an unused-import issue, remove the import statement for the module that is never referenced in the file. This eliminates an unnecessary dependency and has no effect on runtime behavior when the import is truly unused.

In this specific case, the best fix is to delete the import json line at the top of tests/persistence/test_storing_of_progress_in_sqlite.py. No other changes are needed because there are no references to json elsewhere in the provided snippet. You should only modify line 1 of this file, leaving all other imports (sqlite3, pytest, and the kopf imports) unchanged.

Suggested changeset 1
tests/persistence/test_storing_of_progress_in_sqlite.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/tests/persistence/test_storing_of_progress_in_sqlite.py b/tests/persistence/test_storing_of_progress_in_sqlite.py
--- a/tests/persistence/test_storing_of_progress_in_sqlite.py
+++ b/tests/persistence/test_storing_of_progress_in_sqlite.py
@@ -1,4 +1,3 @@
-import json
 import sqlite3
 
 import pytest
EOF
@@ -1,4 +1,3 @@
import json
import sqlite3

import pytest
Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
Comment thread tests/persistence/test_storing_of_progress_in_sqlite.py Outdated
import pytest

from kopf._cogs.configs.progress import ProgressRecord, SQLiteProgressStorage
from kopf._cogs.structs.bodies import Body, BodyEssence

Check notice

Code scanning / CodeQL

Unused import

Import of 'BodyEssence' is not used.

Copilot Autofix

AI 2 months ago

In general, unused imports should be removed to keep dependencies minimal, improve readability, and avoid misleading future maintainers into thinking the imported symbol is needed. In this case, we should update the import on line 7 so that it only imports Body, which is the symbol actually used in this test file.

Concretely, in tests/persistence/test_storing_of_progress_in_sqlite.py, locate the line:

from kopf._cogs.structs.bodies import Body, BodyEssence

and change it to:

from kopf._cogs.structs.bodies import Body

No other code changes or additional imports are required, since Body is already used and BodyEssence is not referenced anywhere in the shown code.

Suggested changeset 1
tests/persistence/test_storing_of_progress_in_sqlite.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/tests/persistence/test_storing_of_progress_in_sqlite.py b/tests/persistence/test_storing_of_progress_in_sqlite.py
--- a/tests/persistence/test_storing_of_progress_in_sqlite.py
+++ b/tests/persistence/test_storing_of_progress_in_sqlite.py
@@ -4,7 +4,7 @@
 import pytest
 
 from kopf._cogs.configs.progress import ProgressRecord, SQLiteProgressStorage
-from kopf._cogs.structs.bodies import Body, BodyEssence
+from kopf._cogs.structs.bodies import Body
 from kopf._cogs.structs.ids import HandlerId
 from kopf._cogs.structs.patches import Patch
 
EOF
@@ -4,7 +4,7 @@
import pytest

from kopf._cogs.configs.progress import ProgressRecord, SQLiteProgressStorage
from kopf._cogs.structs.bodies import Body, BodyEssence
from kopf._cogs.structs.bodies import Body
from kopf._cogs.structs.ids import HandlerId
from kopf._cogs.structs.patches import Patch

Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
@nolar nolar changed the title Store the progress & diff base in file and sqlite storages Store the progress & diff base in file and sqlite storages [v2] Mar 2, 2026
github-advanced-security[bot]
github-advanced-security AI found potential problems Mar 9, 2026
View reviewed changes
Copy link
Copy Markdown

@github-advanced-security github-advanced-security AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@nolar nolar force-pushed the more-storages branch 11 times, most recently from 5dfa6e2 to 8f1e459 Compare March 24, 2026 14:13
@nolar nolar mentioned this pull request Mar 27, 2026
Open
nolar and others added 9 commits April 1, 2026 13:36
@nolar
Span core tasks over the cleanup activities for symmetry
f672fb5 
The "core" tasks (now: only the authenticator) run during the startup activities normally (spawned in parallel), so it makes sense to keep them during the cleanup activities. This also makes the code more symmetric. There is no big difference in terms of the run flow, since the custom (user-side) cleanup activities do not use the K8s API.

Signed-off-by: Sergey Vasilyev <nolar@nolar.info>
@nolar
Reorder pyupgrade and isort, so that they change files in one run
0dc3cc8 
Signed-off-by: Sergey Vasilyev <nolar@nolar.info>
@nolar
Convert storages to async context managers for proper resource manage…
29b0139 
…ment

Signed-off-by: Sergey Vasilyev <nolar@nolar.info>
@nolar
Convert storage methods to async, keep compatible with sync
7fb5ce5 
Signed-off-by: Sergey Vasilyev <nolar@nolar.info>
@nolar
Warn on declaring the sync methods in storages
79120dc 
Signed-off-by: Sergey Vasilyev <nolar@nolar.info>

# Conflicts:
#	kopf/_cogs/configs/diffbase.py
#	kopf/_cogs/configs/progress.py
@nolar @claude
Let custom storages erase the resource-related artifacts on deletion
24a3224 
The new storage method `erase()` accepts only the body kwarg and is a no-op in the base classes (ProgressStorage and DiffBaseStorage), serving as a placeholder for follow-up changes where additional storage types (e.g. file-based, SQLite) will need to clean up their persisted records when an object is gone from the cluster.

The method is not overridden in any existing descendants (annotations- and status-based storages, multi-storages) since they store data on the object itself and have nothing to clean up externally.

In `processing.py`, `erase()` is called on both diffbase_storage and progress_storage when the cause is GONE, i.e. when the object has been deleted from the cluster and the operator is notified for the last time.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Sergey Vasilyev <nolar@nolar.info>
@nolar
Document the possibility of custom storages for progress and diff base
0e499f4 
Signed-off-by: Sergey Vasilyev <nolar@nolar.info>
@nolar
Store diffbase/progress in a shared filesystem or a container volume
c627df6 
Signed-off-by: Sergey Vasilyev <nolar@nolar.info>
@nolar
Store diffbase/progress in a shared sqlite database
15d395b 
Signed-off-by: Sergey Vasilyev <nolar@nolar.info>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nolar @github-advanced-security