Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interface with Contexts? #154

Closed
steve-gray opened this issue Oct 4, 2021 · 9 comments
Closed

Interface with Contexts? #154

steve-gray opened this issue Oct 4, 2021 · 9 comments

Comments

@steve-gray
Copy link

Small feature request - Would it be possible to extend all of the interface operations to have context parameters on a "WithContext" version of the interface, so that repositories/stores can implement contracts that have a context, such as database backed stores? We're finding a little bit tricky at times and having to basically use context.TODO() as we can get a context from the incoming GRPC calls down to the store inside.

We've toyed with other architectures/solutions, including creating an instance per call and using a globally smuggled context variable, but its' just hard yards.

@aeneasr
Copy link
Member

aeneasr commented Oct 4, 2021

Yes, sounds good! Would you be up for a PR? :)

@TabiasP
Copy link
Contributor

TabiasP commented Mar 2, 2023

Is this still a requested feature? We have a forked version that includes the context parameters. We can open a PR if this is still something you all would like to see implemented.

@ccoVeille
Copy link

I don't know for project owners. But I'm interested. So I would love seeing a PR.

@aeneasr
Copy link
Member

aeneasr commented Mar 3, 2023

Yes, contributions welcome! Larger code changes will probably take some time for review, but if it's just adding contexts to some function signatures no biggie :) Glad to see this library still being used :)

This was referenced Mar 6, 2023
@ccoVeille
Copy link

Can we get @TabiasP changes merged?

He implemented the feature in

#159

@TabiasP
Copy link
Contributor

TabiasP commented Mar 4, 2024

Hey all, I've since closed the previous PR and opened a new one. It's been a while since I've taken a look at this but should now be up to date with master and have a passing build: #163

@aeneasr
Copy link
Member

aeneasr commented Mar 5, 2024

Merged

@aeneasr aeneasr closed this as completed Mar 5, 2024
@ccoVeille
Copy link

Thanks a lot @TabiasP

Question for @aeneasr : do you plan to release this in a tagged version?

@ccoVeille
Copy link

released in 1.3.0, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants