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

HTTP and Cache Dependency Design #470

Closed
Lakelezz opened this issue Jan 24, 2019 · 3 comments
Closed

HTTP and Cache Dependency Design #470

Lakelezz opened this issue Jan 24, 2019 · 3 comments
Labels
opinions wanted Discussion or solution to a problem requested. question A question about Serenity.
Milestone

Comments

@Lakelezz
Copy link
Contributor

Lakelezz commented Jan 24, 2019

Foreword

This issue decides how Serenity will provide HTTP and Cache in v0.6.x and marks an important change in the API.

HTTP and Cache Globals

First and foremost, Serenity v0.5.x uses lazy_static to construct global variables that can be accessed from anywhere inside the running application.
Therefore, every Serenity program is limited to only one bot as only one login can be established.

Alternative to Globals

The idea is to have one Cache and one HTTP-client inside the actual Serenity-Client enabling users to have multiple bots in one running application.

Possibilities

We have evaluated the possibilities, one is about passing the dependencies (HTTP and Cache) to needed places and the other one adds a reference inside every stateful type.

Passing

Cache and HTTP will be in every Context you receive.
Stateless types (e.g. Id-structs like ChannelId, GuildId, ...) and stateful types (PartialGuild, Member, ...) will require the user to pass Http, Cache, or the entire Context if a method can utilise the Cache.

Example:
ChannelId(u64).say("Hello");
turns into
ChannelId(u64).say(&context.http, "Hello");

Advantage: No memory-overhead, API shows how methods can access HTTP and Cache (hidden with globals).
Disadvantage: API gets becomes more verbose/harder to use.

Deserialising into stateful Types

When we receive data from Discord Serenity bundles Cache and HTTP and deserialises them into stateful types.
Literally: Every stateful type would own a reference to the actual instance of HTTP and Cache bundle.

Advantage: Same API on stateful types (stateless types' API remains like in the first option).
Disadvantage: 8 Bytes overhead for every stateful type - this overhead may be noticeable when caching.

Your Opinion

At the moment, v0.6.x uses the first variant, we are considering whether we should remain like this, use the deserialised approach, or even revert back to globals, or maybe something entirely different?
Therefore, we require your thoughts and opinions: Should Serenity stay global-oriented or go for one of the non-global approaches?

@Lakelezz Lakelezz added question A question about Serenity. opinions wanted Discussion or solution to a problem requested. labels Jan 24, 2019
@Lakelezz Lakelezz mentioned this issue Jan 27, 2019
18 tasks
@Celti
Copy link
Contributor

Celti commented Feb 19, 2019

Having experimented a bit with the first variant as currently implemented in 0.6.x, I think it works fairly well, and getting rid of globals is a worthy endeavour.

There's a bit of serious cognitive impact, though, on remembering what takes a full Context (e.g., Message::reply()) versus what takes just an Http (e.g., ChannelId::say). Things that only take Cache tend to be well-indicated by the name, but there are a few with quirks there too.

The ergonomics might be helped if such things simply took Context in general, or perhaps Into<Http>?

@Lakelezz
Copy link
Contributor Author

Yes, that is the plan! If we go with passing-approach all functions that require either HTTP, Cache, or both will expect Context to be passed.

@Lakelezz Lakelezz added this to the 0.6 milestone Feb 24, 2019
@Lakelezz
Copy link
Contributor Author

We came to the conclusion passing Context around is the way to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
opinions wanted Discussion or solution to a problem requested. question A question about Serenity.
Projects
None yet
Development

No branches or pull requests

2 participants