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

Async extract for KeyExtractor #44

Open
kr45732 opened this issue Jun 20, 2023 · 7 comments
Open

Async extract for KeyExtractor #44

kr45732 opened this issue Jun 20, 2023 · 7 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@kr45732
Copy link

kr45732 commented Jun 20, 2023

Hello! Would it be possible to support an async extract for the key extractor? I would like to access a tokio mutex when extracting the key. I cannot use the blocking lock as I am using an async runtime. Thanks!

Also, it would be great if the recent key whitelist additions could be pushed to crates.io :)

@AaronErhardt AaronErhardt added enhancement New feature or request help wanted Extra attention is needed labels Jun 20, 2023
@AaronErhardt
Copy link
Owner

Sounds like a good idea. I don't have time to address this issue though. I just review and merge PRs currently.

Also, it would be great if the recent key whitelist additions could be pushed to crates.io :)

I just published 0.4.1 :)

@TheAwiteb
Copy link
Contributor

Hello @AaronErhardt I hope you are well. What do you think about adding a feature called "async" that converts everything to async? Or do you have another suggestion?

@AaronErhardt
Copy link
Owner

Hello @AaronErhardt I hope you are well. What do you think about adding a feature called "async" that converts everything to async? Or do you have another suggestion?

Cargo features should only be additive (adding features should not break your existing code) which means converting is not the right approach. We could however use two feature flags "sync" and "async" where each of them enables a certain part of the code. On the other side, since actix is already async, we could just make async key extractors the only option.

Overall, I think the best approach would be to first implement an async key extractor based on the current code and see how the diff looks before deciding whether to use (which) feature flags.

@TheAwiteb
Copy link
Contributor

which means converting is not the right approach

It adds a feature to the project to make it asynchronous right?

since actix is already async, we could just make async key extractors the only option

But this will make a breaking changes, and we have just released 0.5.0.

I think it is right that we make the "async" feature because it will provide an async support for those who need it and won't make any breaking changes either. As for the "async" and "sync" features, this is not actually correct action, because firstly, they will make breaking changes, and secondly, if none of them are activated, this will be a malfunction (we can do "sync" or "async" if none of them are activated, but it is not with correct use)

@AaronErhardt
Copy link
Owner

It adds a feature to the project to make it asynchronous right?

If that's what you meant, that's correct. You used the word "converting" which can be interpreted as converting existing traits or function signatures to use async, which would be wrong.

We can also just add an "async" feature and make the sync code compile unconditionally to avoid breaking changes. But releasing 0.6 wouldn't be too big of a deal either.

Regardless of all the discussions about features, we first need some sort of basic implementation and look how the changes affect the API. If we reached that point, it should be easy to figure out the best feature flags.

@TheAwiteb
Copy link
Contributor

we first need some sort of basic implementation and look how the changes affect the API

Can you assign it to me please

@TheAwiteb TheAwiteb removed their assignment Dec 14, 2024
@TheAwiteb
Copy link
Contributor

Sorry I can't work on this, I haven't been using Actix for a while

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants