Skip to content

Conversation

@AndrewSisley
Copy link
Contributor

Relevant issue(s)

Resolves #3704

Description

Allows the retrieval of txn from ctx everywhere. Does not remove the many function params that could/should be removed because of this change (we can do that gradually over time).

Is annoying and messy to have to pass both independently into most functions in our codebase, especially when one lives within the other.

Broken out of #3699

@AndrewSisley AndrewSisley added this to the DefraDB v0.18 milestone May 14, 2025
@AndrewSisley AndrewSisley requested a review from a team May 14, 2025 16:55
@AndrewSisley AndrewSisley self-assigned this May 14, 2025
@AndrewSisley AndrewSisley added area/db-system Related to the core system related components of the DB refactor This issue specific to or requires *notable* refactoring of existing codebases and components labels May 14, 2025
@AndrewSisley AndrewSisley force-pushed the 3704-txn-from-ctx branch 2 times, most recently from c158bda to c427213 Compare May 14, 2025 21:39
Copy link
Collaborator

@fredcarle fredcarle left a comment

Choose a reason for hiding this comment

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

LGTM. The only thing that I don't like about this is the package name. I'm wondering if we should have a ctx package and have most related things in there. Like the id caches and the context init function.

@AndrewSisley
Copy link
Contributor Author

AndrewSisley commented May 15, 2025

The only thing that I don't like about this is the package name

Yeah, it's not ideal - I called it txn at first but it of course was shadowed by everything. ctx would have the same problem.

I'm wondering if we should have a ctx package and have most related things in there. Like the id caches and the context init function.

I'm currently against that as it forces unrelated stuff into the same space and IMO goes against the spirit of ctx. It would however allow us to create a strongly typed ctx, but I'm still very weary of the coupling it would introduce.

@AndrewSisley AndrewSisley merged commit 2c67d8d into sourcenetwork:develop May 15, 2025
41 of 43 checks passed
@AndrewSisley AndrewSisley deleted the 3704-txn-from-ctx branch May 15, 2025 13:56
ChrisBQu pushed a commit to ChrisBQu/defradb that referenced this pull request Jun 16, 2025
)

## Relevant issue(s)

Resolves sourcenetwork#3704

## Description

Allows the retrieval of txn from ctx everywhere. Does not remove the
many function params that could/should be removed because of this change
(we can do that gradually over time).

Is annoying and messy to have to pass both independently into most
functions in our codebase, especially when one lives within the other.

Broken out of sourcenetwork#3699
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/db-system Related to the core system related components of the DB refactor This issue specific to or requires *notable* refactoring of existing codebases and components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow fetching of txn from ctx anywhere

2 participants