Cache storage abstraction #6052

kajarenc · 2023-02-02T16:46:18Z

📚 Context

Introduce the new CacheStorage and CacheStorageManager abstractions for @st.cache_data. This allows to customize the way that cache values are stored within Streamlit.

RuntimeConfig now takes a CacheStorageManager interface, and cache_data_api constructs the CacheStorage using that instance.

We ship with a concrete LocalDiskCacheStorage and LocalDiskCacheStorageManager
implementation, which just stores cache items in memory and on disk if persist=True is passed to st.cache_data (as before). This is what server.py uses when it initializes its Runtime instance.

We also implemented InMemoryCacheStorageWrapper class, that could serve as facade and wrap any CacheStorage with in_memory first layer cache.

Also, for testing purposes, DummyCacheStorage and DummyCacheStorageManagerimplemented.

Because the LocalDiskCacheStorageManager instance exists when a Runtime has been created, scripts that run in "raw" mode (i.e. streamlit._is_running_with_streamlit == False) no longer have access to that manager from Runtime. So in get_storage_manager in cache_data_api we fallback to DummyCacheStorageManager.

What kind of change does this PR introduce?
- Bugfix
- Feature
- Refactoring
- Other, please describe:

🧠 Description of Changes

Add bullet points summarizing your changes here
- This is a breaking API change
- This is a visible (user-facing) change

Revised:

Insert screenshot of your updated UI/code here

Current:

Insert screenshot of existing UI/code here

🧪 Testing Done

Screenshots included
Added/Updated unit tests
Added/Updated e2e tests

🌐 References

Does this depend on other work, documents, or tickets?

Issue: Closes #XXXX

Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

lib/streamlit/runtime/caching/storage/cache_storage.py

lib/streamlit/runtime/caching/cache_data_api.py

…rages. Preparation to rename CacheStorageFactory to Manager

lib/streamlit/runtime/caching/storage/cache_storage_protocol.py

… and max_entries to use also None param

lib/streamlit/runtime/caching/cache_utils.py

+def ttl_to_seconds(
+    ttl: float | timedelta | None, *, allow_none: Literal[True]
+) -> float | None:
+    ...


lib/streamlit/runtime/caching/cache_utils.py

sfc-gh-jcarroll · 2023-02-14T22:40:49Z

lib/streamlit/runtime/caching/storage/cache_storage.py

+_CACHED_FILE_EXTENSION = "memo"
+
+
+class OpenSourceCacheStorageManager(CacheStorageManager):


Just a suggestion, maybe we call it LocalCacheStorageManager

sfc-gh-jcarroll · 2023-02-14T22:56:26Z

lib/streamlit/runtime/caching/storage/cache_storage_protocol.py

+        """
+        pass
+
+    def get_stats(self) -> list[int]:


Per discussion earlier, propose we remove this from the interface

# Conflicts: # lib/streamlit/runtime/caching/cache_data_api.py # lib/streamlit/runtime/caching/cache_utils.py

…rage to LocalCacheStorage

…StorageCache

…d in memory

lib/streamlit/runtime/caching/storage/dummy_cache_storage.py

…in raw mode (using InMemoryWrappedDummyCacheStorageManager

kajarenc · 2023-02-24T18:28:44Z

lib/streamlit/web/cli.py

-    caching.cache_data.clear()
+    cache_storage_manager = get_cache_storage_manager()
+    cache_storage_manager.clear_all()
    caching.cache_resource.clear()


Actually, streamlit cache clear works from another python process and doesn't have access to in_memory cache (that was true for previous implementation, and remains true for current).

It removes only persist cache (in our case only files from disk)

Related to this maybe we need to remove caching.cache_resource.clear() call from here at all, and also rework documentation to not mislead users.

Yes, we should not change the behavior or add anything new or complex here. If it seems like any regression lets discuss (but sounds like it's not any).

Agree if we can easily update the docs / code to be more clear about the behavior, that's a nice to have.

tconkling

Looks great! Mostly just nits, but I also have some questions about thread safety notes that I think are maybe just a misunderstanding, but want to make sure.

Also, I wonder if we need to think more thoroughly about concurrency in our local disk storage. We are probably fine (and I don't think any of these changes introduce any new local disk concurrency issues that weren't already present) but... let's verify that assumption.

tconkling · 2023-02-24T20:16:39Z

lib/streamlit/runtime/caching/cache_data_api.py

            allow_widgets=self.allow_widgets,
        )

+    def validate_params(self):


Nit: can you add a return type annotation?

tconkling · 2023-02-24T20:18:01Z

lib/streamlit/runtime/caching/cache_data_api.py

        )

+    def validate_params(self):
+        """ "


Nits: stray " here; also start of the docstring should go next to the opening """.

Can you add specificity around what it means to validate params? What happens if validation fails? Do we throw an Exception? If so, what type of Exception?

tconkling · 2023-02-24T20:20:22Z

lib/streamlit/runtime/caching/cache_data_api.py

+                # try to remove in optimal way; if not possible, fallback to remove all
+                # available storages one by one
+                self.get_storage_manager().clear_all()


Can you extend this slightly to explain why we want this fallback? (Why don't we trust storage manager to do the right thing?)

Yep, reworked comment.
Basically clear_all method of CacheStorageManager is not required to implement (e.g. when there is no good way to delete all storages at once), so we should have alternative mechanism to remove at least all storages that we have in memory

tconkling · 2023-02-24T20:20:53Z

lib/streamlit/runtime/caching/cache_data_api.py

+        max_entries: int | None,
+        ttl: int | float | timedelta | None,
+    ) -> None:
+        """Validate that the cache params are valid for given storage."""


What happens if validation fails?

Now CacheStorageManager has ability to raise CacheStorageImproperlyConfigured in check_context method, and here we catch that error, log it, and then propagate it .

tconkling · 2023-02-24T20:37:40Z

lib/streamlit/runtime/caching/storage/__init__.py

+from streamlit.runtime.caching.storage.cache_storage_protocol import (
+    CacheStorage as CacheStorage,
+)
+from streamlit.runtime.caching.storage.cache_storage_protocol import (
+    CacheStorageContext as CacheStorageContext,
+)
+from streamlit.runtime.caching.storage.cache_storage_protocol import (
+    CacheStorageError as CacheStorageError,
+)
+from streamlit.runtime.caching.storage.cache_storage_protocol import (
+    CacheStorageKeyNotFoundError as CacheStorageKeyNotFoundError,
+)
+from streamlit.runtime.caching.storage.cache_storage_protocol import (
+    CacheStorageManager as CacheStorageManager,
+)


Nit: I think this is more verbose than it needs to be - can we combine these imports?

from streamlit.runtime.caching.storage.cache_storage_protocol import ( CacheStorage as CacheStorage, CacheStorageContext as CacheStorageContext, ... )

I changed it as suggetsted, but pre-commit hooks (isort?) always revert it back

tconkling · 2023-02-24T21:16:21Z

lib/streamlit/web/cache_storage_manager_config.py

+)
+
+
+def get_cache_storage_manager() -> CacheStorageManager:


Nit: this unconditionally creates a new CacheStorageManager, but the name makes it sound like it just gets an existing one. Can we change it to create_default_cache_storage_manager or something?

tconkling · 2023-02-24T21:17:46Z

lib/tests/streamlit/runtime/app_session_test.py

        mock_runtime.media_file_mgr = MediaFileManager(
            MemoryMediaFileStorage("/mock/media")
        )
+        # TODO: Add a mock for CacheStorageManager


Is this intended to be a TODO? Can we do it now?

tconkling · 2023-02-24T21:46:43Z

lib/tests/streamlit/runtime/caching/storage/in_memory_cache_storage_wrapper_test.py

+local_disk_cache_storage = LocalDiskCacheStorage(context)
+dummy_cache_storage = DummyCacheStorage()


Nit: can we remove these from the file scope? It looks like they're just used in test_in_memory_cache_storage_wrapper_works_with_any_storage - can they just be declared inline there?

done, refactored to different tests

tconkling · 2023-02-24T21:47:07Z

lib/tests/streamlit/runtime/caching/storage/in_memory_cache_storage_wrapper_test.py

+    LocalDiskCacheStorage,
+)
+
+context = CacheStorageContext(


Nit: all-caps constant names (CONTEXT)

tconkling · 2023-02-24T21:49:49Z

lib/tests/streamlit/runtime/caching/storage/local_disk_cache_storage_test.py

+        self.patch_get_cache_folder_path.stop()
+        self.tempdir.cleanup()
+
+    def test_create_persist_context(self):


Can you add short docstrings to this and the rest of the tests in the file explaining what's being tested?

* Add CacheStorageImproperlyConfigured exception

* other renaming and docstring addition based on review comments

…s not implemented by design

tconkling

Looks great! My only real complaint is with the name "DummyCacheStorageManager", which confused me until I looked closely at the code and realized that it's just an in-memory cache with no persistence.

tconkling · 2023-02-28T19:39:09Z

lib/streamlit/runtime/caching/cache_data_api.py

+        Validate the params passed to @st.cache_data are compatible with cache storage
+
+        When called, this method could log warnings if cache params are invalid
+        for current storage.


It can also throw a specific exception, right?

tconkling · 2023-02-28T19:41:37Z

lib/streamlit/runtime/caching/storage/cache_storage_protocol.py

+    """Raised when the key is not found in the cache storage"""
+
+
+class CacheStorageImproperlyConfigured(CacheStorageError):


Can we change this name to something like InvalidCacheStorageContext, to make clear that it's specifically for validating a CacheStorageContext?

tconkling · 2023-02-28T19:44:02Z

lib/streamlit/runtime/caching/storage/local_disk_cache_storage.py

+class LocalDiskCacheStorage(CacheStorage):
+    """
+    Cache storage that persists data to disk
+    This is the default cache storage for @st.cache_data


Nit: "This is the default cache persistence layer for @st.cache_data" (because it will always have a memory cache layer in front of it, right?)

@tconkling, @vdonato I would like to explain the original idea:

Storage should not know about any details of its use, and only provide the ability to add and remove entries.

In this case, the LocalDiskCacheStorage class purely provides the ability to work with disk storage.
It is only the responsibility of LocalDiskCacheStorage class.

In contrary LocalDiskCacheStorageManager is responsible for creating those storages, and in our case, it is wrapping those storage instances with another layer of in-memory cache via InMemoryCacheWrapper. LocalDiskCacheStorageManager was originally called InMemoryWrappedLocalDiskCacheStorageManager :) but then during a review, we renamed that to LocalDiskCacheStorage for simplicity).

So the idea is that each implementation should consist of ConcreteCacheStorage, and ConcreteCacheStorageManager , responsible for the creation (and optional decoration) of ConcreteCacheStorage.

For convenience, we provide InMemoryCacheStorageWrapper class, which could wrap any storage with an in-memory cache layer, but this is a detail of the manager, not the storage itself.

I think this is my flaw in the description of the classes, it will be necessary to work on this improvement

Yeah, I get that - I'm just nitpicking the comment. "This is the default cache storage for @st.cache_data" sounds to me like it means "by default, we use only the local disk for cache storage."

But that's not the case: by default, we use memory for cache storage, with LocalDiskCacheStorage as the persistence layer for functions that opt-in to persistence.

oh, get it :) comment rephrased!

tconkling · 2023-02-28T21:52:58Z

lib/streamlit/runtime/caching/storage/dummy_cache_storage.py

+)
+
+
+class DummyCacheStorageManager(CacheStorageManager):


I think this class is misnamed - "DummyCacheStorageManager" makes it sound like the cache is a no-op. But actually, this is just a memory-only cache storage with no persistence layer. Can we call it "MemoryCacheStorageManager" instead?

Seems reasonable-ish to me since (assuming I'm understanding the code correctly) this is only used to handle the case where a user calls python3 my_script.py and we want the script run to essentially be a no-op

I do think this could benefit from a class docstring mentioning what it's used for so that it isn't confused for a manager implementation that's used when running a Streamlit server, though

vdonato

Leaving a drive-by review even if we're already covered for this PR since this caching project interests me 👀

LGTM! I'll avoid officially +1-ing this myself since Tim already has / I'm not confident enough in my knowledge of the caching code to want to click approve

I left some comments that are mostly nits + naming suggestions to potentially make things easier on a reader that's never looked at any of this code before.

Also, I think it'd be nice to have a diagram on how caching works since it took some effort to understand what's going on here as a first-time reader of most of this code. I think an ASCII-art (or nicer if someone's willing to put in the effort) class diagram or even just a text-description of how all of the classes interact (like what we have in event_based_path_watcher.py) would help out in the ramp-up process for this part of the codebase a ton given how many layers of indirection are involved.

vdonato · 2023-03-01T05:45:00Z

lib/streamlit/runtime/caching/cache_utils.py

+def ttl_to_seconds(
+    ttl: float | timedelta | None, *, allow_none: bool = False
+) -> float | None:
+    """
+    Convert a ttl value to a float representing "number of seconds".
    """
-    if ttl is None:
+    if not allow_none and ttl is None:
        return math.inf


nit: could the allow_none param be renamed to something like coerce_none_to_inf that defaults to True? (I don't particularly like the suggested name either but can't think of anything better at the moment)

allow_none makes me think that an error is raised if None is passed to the function

done, renamed to coerce_none_to_inf :)

vdonato · 2023-03-01T05:47:11Z

lib/streamlit/runtime/caching/cache_data_api.py


+    def validate_params(self) -> None:
+        """
+        Validate the params passed to @st.cache_data are compatible with cache storage


nit (here and throughout the PR):

Conventionally, the summary of a docstring is placed on the same line as the opening triple quotes

See https://peps.python.org/pep-0008/#documentation-strings

vdonato · 2023-03-01T07:56:29Z

lib/streamlit/web/cache_storage_manager_config.py

+)
+
+
+def create_default_cache_storage_manager() -> CacheStorageManager:


It feels weird to me that we have a function that just calls the LocalDiskCacheStorageManager constructor. Couldn't we just call the constructor directly where we need it?

I think this function adds clarity: callers are specifically saying "I want whatever the default is". (That happens to be LocalDiskCacheStorageManager right now, but calling the constructor directly wouldn't communicate its default-ness.)

vdonato · 2023-03-01T07:59:11Z

lib/streamlit/web/cli.py

+    cache_storage_manager = create_default_cache_storage_manager()
+    cache_storage_manager.clear_all()


nit: It may be useful to add a comment here pointing out that since this is a separate CLI command, there's no runtime/streamlit server/etc. and that's why it's reasonable to create a new cache manager here.

Potentially an obvious call-out, but I was confused as to why we were creating a new instance for a second when initially reading this code

an explanatory comment added!

vdonato · 2023-03-01T08:05:15Z

lib/streamlit/runtime/caching/storage/cache_storage_protocol.py

+    persist: Literal["disk"] | None = None
+
+
+class CacheStorage(Protocol):


I wonder if this would be better named something like FunctionCacheStorage to emphasize that it's the cache for a single annotated function. It wasn't clear to me why we have so many layers of indirection in this code until I had finished reading the docstrings for each of the classes involved.

This may be because I'm reviewing this code without the deepest knowledge of our caching internals, but there are enough layers involved that I think it may help if it doesn't make things too verbose.

I absolutely see your point, we definitely have too many layers of indirections for caching.
In cache_utils.py we have Cache, CachedFunc, and CachedFuncInfo classes, as well as DataCaches and DataCache classes in cache_data_api.py to implement all machinery required for keeping the ability to decorate functions.

That's the reason I tried to abstract the layer responsible for reading/writing results from that machinery of function decoration. This CacheStorage and CacheStorageManager was designed keeping the idea in mind that they should not know anything about what and how were caches, working purely with keys and bytes.

It wasn't 100% done, e.g. CacheStorageContext should know
about function_key (to prefix files), and function_display_name to raise exceptions with that name, but at least our storage doesn't know anything about function entity, params hashing algorithms, etc.

I agree that the motivation for why we have many CacheStorages per function instead of one should be better explained in the documentation/diagrams, but I want to keep the CacheStorage name as is since it should be not connected with the function.

Seems reasonable -- I think my response in this case would be to push a bit harder for a class diagram explaining how all these things work together so that it's more clear to the reader before they've read through all of the code/docstrings + put some work into figuring it out

vdonato · 2023-03-01T08:17:25Z

lib/streamlit/runtime/caching/storage/dummy_cache_storage.py

+)
+
+
+class DummyCacheStorageManager(CacheStorageManager):


Seems reasonable-ish to me since (assuming I'm understanding the code correctly) this is only used to handle the case where a user calls python3 my_script.py and we want the script run to essentially be a no-op

I do think this could benefit from a class docstring mentioning what it's used for so that it isn't confused for a manager implementation that's used when running a Streamlit server, though

vdonato · 2023-03-01T08:28:06Z

lib/streamlit/runtime/caching/cache_data_api.py

+        ttl_seconds: float | None,
+        max_entries: int | None,
+    ) -> CacheStorageContext:
+


nit: extra newline

vdonato · 2023-03-01T08:52:44Z

lib/streamlit/runtime/caching/storage/cache_storage_protocol.py

+        meaningful default behaviour is to raise NotImplementedError, so this is not
+        abstractmethod.


nit: It may be a bit more clear to say that the method is optional in this docstring. I think it's mentioned above already but repeating it here would be good

added

The method is optional to implement: cache data API will fall back to remove all available storages one by one via storage.clear() method if clear_all raises NotImplementedError.

paragraph

vdonato · 2023-03-01T08:56:58Z

lib/streamlit/runtime/caching/storage/cache_storage_protocol.py

+        """
+        raise NotImplementedError
+
+    def check_context(self, context: CacheStorageContext) -> None:


nit: validate_context might be a nicer name to match with the validate_* methods that live in cache_data_api.py

yeа, I also don't like that we have inconsistencies with the validate_ and check_ naming.
in this case, I would like to save the check_context in the public protocol, because this method should not produce/return any validated data, but just log a message/raise an exception.

* more tests added

…heStorageManager) to MemoryCacheStorageManager

…ocol

lib/streamlit/runtime/caching/cache_utils.py

 def ttl_to_seconds(ttl: float | timedelta | None) -> float:
-    """Convert a ttl value to a float representing "number of seconds".
-    If ttl is None, return Infinity.
+    ...


defining protocol

b900222

github-advanced-security bot found potential problems Feb 2, 2023

View reviewed changes

lib/streamlit/runtime/caching/storage/cache_storage.py Fixed Show fixed Hide fixed

add custom exceptions for cache storage protocol

7e629d2

kajarenc force-pushed the cache-storage-protocol branch from 4e54e3c to 7e629d2 Compare February 6, 2023 18:58

draft version, teach DataCache and DataCaches work with storage protocol

41fe3b9

github-advanced-security bot found potential problems Feb 7, 2023

View reviewed changes

lib/streamlit/runtime/caching/cache_data_api.py Fixed Show fixed Hide fixed

lib/streamlit/runtime/caching/cache_data_api.py Fixed Show fixed Hide fixed

lib/streamlit/runtime/caching/cache_data_api.py Fixed Show fixed Hide fixed

kajarenc added 8 commits February 8, 2023 21:13

use runtime dependency injection for concrete cache storage factory

8d599bd

Add clear_all method to CacheStorageFactory for optimal clear all sto…

57fee0c

…rages. Preparation to rename CacheStorageFactory to Manager

rename CacheStorageFactory to CacheStorageManager

446723b

remove dummy return value by empty list

7c8f695

implement get_stats for default cache storage

3e58fb3

Add TODO comment

bd34bfd

Merge branch 'develop' into cache-storage-protocol

509c08f

remove unused WORK_WITH_PERSIST_KEYWORD from protocol

4472694

github-advanced-security bot found potential problems Feb 13, 2023

View reviewed changes

lib/streamlit/runtime/caching/storage/cache_storage_protocol.py Fixed Show fixed Hide fixed

Add check_context method to protocol, fix mypy errors, refactor ttl…

440e7d7

… and max_entries to use also None param

github-advanced-security bot found potential problems Feb 13, 2023

View reviewed changes

kajarenc added 2 commits February 14, 2023 18:56

Fixes/additions in docstrings

03e6782

Add more docstrings

a20f916

sfc-gh-kjavadyan changed the title ~~[WIP] cache storage protocol~~ Cache storage protocol Feb 14, 2023

sfc-gh-jcarroll reviewed Feb 14, 2023

View reviewed changes

kajarenc added 5 commits February 15, 2023 19:46

Merge branch 'develop' into cache-storage-protocol

05bbcf2

# Conflicts: # lib/streamlit/runtime/caching/cache_data_api.py # lib/streamlit/runtime/caching/cache_utils.py

remove get_stats from CacheStorageProtocol, rename OpenSourceCacheSto…

78f8b1d

…rage to LocalCacheStorage

Separate local cache storage logic into InMemoryWrapper and LocalDisk…

cf7f8f6

…StorageCache

Always try to call second persistent cache storage if result not foun…

b922f9f

…d in memory

Make most of the old tests work

048f9ac

github-advanced-security bot found potential problems Feb 18, 2023

View reviewed changes

kajarenc added 2 commits February 20, 2023 21:30

Fix existing tests

f7eade4

Fix existing tests, add ability to run scripts with cache_data calls …

6d39b8f

…in raw mode (using InMemoryWrappedDummyCacheStorageManager

kajarenc added 2 commits February 24, 2023 19:16

Add tests for in_memory_cache_storage_wrapper.py

58121f7

Add tests for InMemoryWrappedLocalDiskCacheStorageManager

23e5cd4

kajarenc changed the title ~~Cache storage protocol~~ Cache storage abstraction Feb 24, 2023

kajarenc marked this pull request as ready for review February 24, 2023 17:00

fix minor documentation nits

8281ce9

kajarenc commented Feb 24, 2023

View reviewed changes

tconkling suggested changes Feb 24, 2023

View reviewed changes

kajarenc added 5 commits February 27, 2023 16:36

small fixes after review

af66560

Merge branch 'develop' into cache-storage-protocol

28d24d3

* Improve thread safety related docstrings

6c75382

* Add CacheStorageImproperlyConfigured exception

* remove InMemoryWrapper suffix from storage and manager class names

f2e70b1

* other renaming and docstring addition based on review comments

* remove .clear_all() call for dummy cache storage manager since it i…

d7c086f

…s not implemented by design

tconkling approved these changes Feb 28, 2023

View reviewed changes

vdonato reviewed Mar 1, 2023

View reviewed changes

kajarenc added 4 commits March 1, 2023 18:40

rename CacheStorageImproperlyConfigured to InvalidCacheStorageContext

72e8427

* rename allow_none to coerce_none_to_inf for ttl_to_seconds function

0a2c7df

* more tests added

* rename DummyCacheStorageManager (previously InMemoryWrappedDummyCac…

18d7b3e

…heStorageManager) to MemoryCacheStorageManager

* Documentation changes, added module docstring to cache_storage_prot…

dc3ff59

…ocol

github-advanced-security bot found potential problems Mar 1, 2023

View reviewed changes

kajarenc added the security-assessment-completed Security assessment has been completed for PR label Mar 2, 2023

kajarenc added 3 commits March 2, 2023 20:15

docstring fix

a4029f2

remove redundant newline

1238208

Added more docstrings and explanatory comments

3cf8489

kajarenc merged commit 17e5328 into streamlit:develop Mar 2, 2023

vdonato mentioned this pull request Mar 14, 2023

Bug in clearing cache of cached function via .clear() in streamlit==1.20.0 #6310

Closed

5 tasks

vdonato mentioned this pull request Mar 24, 2023

Set a default for RuntimeConfig.cache_storage_manager #6361

Merged

1 task

vdonato mentioned this pull request Apr 16, 2023

Bring your own Redis cache #5849

Open

		_CACHED_FILE_EXTENSION = "memo"


		class OpenSourceCacheStorageManager(CacheStorageManager):

		local_disk_cache_storage = LocalDiskCacheStorage(context)
		dummy_cache_storage = DummyCacheStorage()

		"""Raised when the key is not found in the cache storage"""


		class CacheStorageImproperlyConfigured(CacheStorageError):

		)


		def create_default_cache_storage_manager() -> CacheStorageManager:

		cache_storage_manager = create_default_cache_storage_manager()
		cache_storage_manager.clear_all()

		persist: Literal["disk"] \| None = None


		class CacheStorage(Protocol):

		meaningful default behaviour is to raise NotImplementedError, so this is not
		abstractmethod.

Cache storage abstraction #6052

Cache storage abstraction #6052

Uh oh!

Conversation

kajarenc commented Feb 2, 2023 • edited by sfc-gh-kjavadyan Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📚 Context

🧠 Description of Changes

🧪 Testing Done

🌐 References

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Check notice

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kajarenc Feb 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tconkling left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kajarenc Feb 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tconkling left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kajarenc commented Feb 2, 2023 •

edited by sfc-gh-kjavadyan

Loading

kajarenc Feb 24, 2023 •

edited

Loading

kajarenc Feb 28, 2023 •

edited

Loading

vdonato Mar 1, 2023 •

edited

Loading

vdonato Mar 1, 2023 •

edited

Loading