AaronErhardt/actix-governor

Async extract for KeyExtractor

kr45732 opened this issue ยท 7 comments

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 :)

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 :)

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?

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.

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)

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.

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

Can you assign it to me please