transactioner #19854
transactioner #19854
Conversation
altendky
added
Changed
|
|
||
| def generate_in_memory_db_uri() -> str: | ||
| # We need to use shared cache as our DB wrapper uses different types of connections | ||
| return f"file:db_{secrets.token_hex(16)}?mode=memory&cache=shared" |
There was a problem hiding this comment.
Any particular reason for indeterminism here rather than passing in a nonce and deferring to the client?
There was a problem hiding this comment.
secrets...? that's odd. but i think this is just used for testing and simulator. was just copy/pasted in this pr. could well be better as you suggest.
|
|
||
|
|
||
| @dataclass | ||
| class SqliteConnection: |
There was a problem hiding this comment.
I understand you don't like inheritance, but isn't there tangible benefit to not having to do all of the self._connection.whatever() pass through that could miss an endpoint in the future or create surface area for bugs in pass-through?
There was a problem hiding this comment.
counter point is that i explicitly have had interest in having a wrapper layer independent of this change. i forget what else, but i at least considered banning certain sqlite queries etc. i think there were other uses... maybe...
| row_factory: Optional[type[aiosqlite.Row]] = None, | ||
| ) -> AsyncIterator[DBWrapper2]: | ||
| db_uri = generate_in_memory_db_uri() | ||
| async with sqlite_wrapper.managed( |
There was a problem hiding this comment.
I don't know how feasible it is, but it would be cool to have an example of a minimal backend that is not sqlite in this test.
There was a problem hiding this comment.
having common testing against the various implemented backends is one of the major things that is missing so far. vaguely, i expect to have common tests and each backend must implement test-specific activities like 'write to a value' and 'read that value' or somesuch. just whatever the common tests require to run.
github-actions
bot
added
the
merge_conflict
Purpose:
Current Behavior:
New Behavior:
Testing Notes: