-
Notifications
You must be signed in to change notification settings - Fork 128
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
Allow more than one api client per application/move config out of config.exs #134
Comments
Thanks for opening this up! Sorry, but this will be a bit of a read. 🙃 I completely agree that reading from a config file is generally not a good idea for a library, there's even a section on it here in the official elixir docs. That being said, because we're running nostrum as an OTP application I believe there's not much we can do in this department. If you've used something like phoenix before, you should be accustomed to including things like tokens and API keys in your config (although our config system is not nearly as robust). As for the hedwig adapter, in the past someone has tried to hook up nostrum to hedwig with little success, maybe for the reasons you're mentioning here. I'm not keen on the benefit of using something like hedwig or how their adapters are implemented, does it just add abstractions for sending/receiving messages? For your other use case, I'm not sure how TOS-friendly (depending on the implementation) the mentioned use case is. This is something I want to take into account when maintaining this lib. Although any changes we make (if any) as a result of this issue will just be coincidental in allowing this behavior I want to bring the issue to your attention if this was something you were trying to do. Regarding the API client, if you're referring to just the API portion of nostrum i.e the REST-ful portion, it's already possible to do what you mention. If by API you mean the library as an entirety, which is the more likely case, then I'd like to ask what your use case here is. Nostrum isn't like most other libraries in that it has the idea of a All that said, I'm pretty open to this change if there's a solid reason for it. We could move the config into ETS or something of the same ilk, or we could provide a more traditional approach where the user has to start nostrum as a whole under their own supervision tree. Let me know your thoughts on the matter! |
Yeah, I looked into the ToS after opening this issue and it seems you're not supposed (allowed?) to share API keys, so you're right on that front, I've decided to have the "bring your own bot user" bit stick to Twitch for now, as that's clearly not against their ToS looking at other similar services. My use-case for integrating with Hedwig though is that I'm working on a multi-platform (/cross-platform, in the sense of different messaging platforms, not operating systems) "bot as a service" application, which means that I'm going to want to be able to reuse code, for example, between a twitch bot and a discord bot, and any more platforms that will come in the future. Hedwig's architecture lets me do this more easily, and also allows me to easily spawn as many bots as is dictated by the _aaS design. In regards to the That said, if I'm going to be running only one discord bot in the end, it almost sounds like it would be reasonable to create some kind of "virtual discord bot" adapter for Hedwig that would let me treat the discord bot on each server as an entirely different "virtual bot" and dodge this issue entirely, and this might even be necessary if I want people to be able to set up different capabilities and features for the discord bot for their own server, at least within the framework that Hedwig provides. |
I think there's probably a useful middle-ground here. I have a use-case or two that I think of as very mundane, but aren't really possible with Nostrum today. Use Case 1I want to provide a bot that has a web interface for configuration. Think of the 1,001 apps out there that you hit the UI for the first time, configure it, and then it becomes whatever it's supposed to be. This is a very sane and useful feature to have for a Discord bot. Examples: DokuWiki, WordPress, openHAB, etc. Use Case 2I have an existing app, and I'd like to add Discord integration. Right now, it's pretty hard to do this without disruption for anything that doesn't have the bot be its primary use-case. If one want's to make something that has Discord functionality that's optional, it's very difficult given that Nostrum won't even let your app start without significant workarounds. Example: Home Assistant, Beyond20, etc. ProposalWhat I propose is:
This gets you the following behavior:
I'd be glad to implement this if you'd accept it. Probably could turn it around quickly. Embedding ExampleFor a real-world example of something that allows this kind of embedding, you can check out |
I notice the alchemy library has this same issue with global architecture. Is this a common thing with elixir libraries? |
Whether a library should be an OTP Application is dependent on whether it makes more sense to be instance-oriented or reuse a single instance. For You can think of Applications as top-level services within a BEAM runtime, that other Applications may depend on. The catch is just, as this issue is about, that you can only have one copy of a given Application running, and that it can add comprehension overhead to people still learning Elixir. Personally, I don't think that should inform such an important architectural detail as whether to use an Application or not, but I've made my case already. |
Reading from config.exs is bad practice for a library in general, but this both prehibits nostrum from being used to create a hedwig adapter and for a large spectrum of use-cases, including, for example, discord bots as a service, where users can input their own token, etc, and set up a bot under a custom bot account for their server.
The text was updated successfully, but these errors were encountered: