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

Add support for shared global state (similar to Actix's web::Data) #12

Open
donicrosby opened this issue Mar 12, 2022 · 8 comments · May be fixed by #13
Open

Add support for shared global state (similar to Actix's web::Data) #12

donicrosby opened this issue Mar 12, 2022 · 8 comments · May be fixed by #13
Assignees
Labels
enhancement New feature or request

Comments

@donicrosby
Copy link

I've been working on a bot that has some commands that requires a database connection in order to hold some state across restarts.

While having the config being passed is fine for stateless bots. Anything that would require a long running connection it wouldn't be efficient to connect to the database every single message. It would be a huge improvement if there was a way to have a single state store that is stood up at startup that can be accessed by all of the commands.

The current config type must have serialize and deserialize which doesn't work for something like a DB without a huge hack of making a custom deserializer that stands up the DB connection to be stored in the config (if that's even possible)

@MTRNord MTRNord added the enhancement New feature or request label Mar 12, 2022
@MTRNord MTRNord self-assigned this Mar 12, 2022
@MTRNord
Copy link
Owner

MTRNord commented Mar 12, 2022

Hm due to the macro currently used this seems like it is quite a lot harder to do as generics wouldnt work as wanted. I am currently trying to understand how actix solved this, as it seems non trivial.

@MTRNord
Copy link
Owner

MTRNord commented Mar 12, 2022

As Data<Config<'a>> in the command handler would conflict with Data<T> in the macro

@MTRNord
Copy link
Owner

MTRNord commented Mar 12, 2022

To be exact this is the issue the macro currently faces:

error[E0308]: mismatched types
 --> example-bot\src\commands\mod.rs:7:1
  |
7 | #[command_generate(bot_name = "Example", description = "This bot prints hello!")]
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `config::Config`, found type parameter `T`
  |
  = note: expected struct `mrsbfh::utils::Data<config::Config<'_>>`
             found struct `mrsbfh::utils::Data<T>`
  = note: this error originates in the attribute macro `command_generate` (in Nightly builds, run with -Z macro-backtrace for more info)

@MTRNord
Copy link
Owner

MTRNord commented Mar 12, 2022

It seems we would need something like this https://docs.rs/actix-web/latest/src/actix_web/handler.rs.html#124-153 But that seems restricting. I will check how axum does data passing. Maybe that has a nicer solution

@MTRNord
Copy link
Owner

MTRNord commented Mar 12, 2022

It seems the best would be to use a trait approach, where commands implement a trait. Instead of using the proc macro.

@donicrosby
Copy link
Author

My only issue with that is it would break backwards compatibility, but we are still v0 😁

@MTRNord
Copy link
Owner

MTRNord commented Mar 12, 2022

My only issue with that is it would break backwards compatibility, but we are still v0 😁

I dont think I can not break that while implementing this. But I will try to make it non breaking. Also I likely go more for the way axum does it then actix. As it seems easier to implement :)

@MTRNord
Copy link
Owner

MTRNord commented Mar 12, 2022

Also I am trying to still keep the macro and use the traits internally. But the handlers probably will change and possibly also how you init it. But I am still in the experimentation phase and hope to later share a first working iteration.

@MTRNord MTRNord linked a pull request Mar 12, 2022 that will close this issue
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants