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

Issues from Disgord #24

Closed
1 task done
switchupcb opened this issue Jul 25, 2022 · 10 comments
Closed
1 task done

Issues from Disgord #24

switchupcb opened this issue Jul 25, 2022 · 10 comments
Labels
documentation Improvements or additions to documentation

Comments

@switchupcb
Copy link
Owner

switchupcb commented Jul 25, 2022

Disgo solves the following issues from https://github.com/andersfylling/disgord/issues.

@switchupcb switchupcb added the documentation Improvements or additions to documentation label Jul 25, 2022
@switchupcb
Copy link
Owner Author

andersfylling/disgord#26 (by @andersfylling) outlines an issue with a lack of support for OAuth2 flows. Disgo implements and provides steps for all OAuth2 flows in oauth2.go.

@switchupcb
Copy link
Owner Author

andersfylling/disgord#283 (by @Kelwing) addresses an inefficiency with Per Resource Per Route (Emoji) rate limits. This user expects 4 rps per channel, but only receives 1 rps with Disgord for endpoints that use reactions. This issue is explained in #22.

@switchupcb
Copy link
Owner Author

andersfylling/disgord#343 outlines that rate limits should be applied to the entire bot, rather than per the entire bot. Disgo handles this functionality in session.go line 106. However, Sharding is not completely implemented (as per #26).

@switchupcb
Copy link
Owner Author

andersfylling/disgord#81 addresses an issue regarding unit tests with Disgord. Users can unit test Disgo by passing events to respective handler functions. This can occur since each handler function uses it's respective event.

@switchupcb
Copy link
Owner Author

andersfylling/disgord#77 is a developer ergonomics improvement that may be added to the tools package. The avatar example showcases when the base64 data uri scheme is required.

@switchupcb
Copy link
Owner Author

andersfylling/disgord#357 addresses a request for a rate limiting test. Disgo provides a rate limiting test in ratelimit_test.go for Global and Per Route rate limits.

@switchupcb
Copy link
Owner Author

andersfylling/disgord#391 (by @meooow25) outlines an issue with a Per Resource (shared) rate limit. This issue is explained in #22. The actual phenomenon experienced by the user can be considered unavoidable since discovery can only occur once a request has been made. However, after that point, an ideal rate limit implementation should account for the shared Token Bucket. One way to reduce this incident from occuring is through the use of default buckets. For more information, read What is a Rate Limit?.

@switchupcb
Copy link
Owner Author

andersfylling/disgord#418 is addressed in the thread.

@switchupcb
Copy link
Owner Author

andersfylling/disgord#433 is already addressed by the tools module.

@switchupcb
Copy link
Owner Author

andersfylling/disgord#353 addresses rate limits, but this implementation (using time/rate) isn't recommended.

In addition to the details present in "What is a Request?", a rate limiter by the rate package will only provide tokens every x intervals. This can result in a situation where the user wants to send a bulk of requests at the edges of a Token Bucket or Fixed Window reset period, but cannot generate more than 1 token per interval in enough time. As a result, this implementation compromises efficiency to ensure the user doesn't send more tokens than the rate limit. However, this compromise is NOT necessary for a valid rate limit implementation.

In addition, the specific implementation described doesn't allow the user to determine how many requests are sent first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

1 participant