Add more configurations for JDABuilder #705

MinnDevelopment · 2018-07-05T18:54:44Z

Pull Request Etiquette

There are several guidelines you should follow in order for your
Pull Request to be merged.

I have checked the PRs for upcoming features/bug fixes.
I have read the contributing guidelines.

It is sometimes better to include more changes in a single commit.
If you find yourself having an overwhelming amount of commits, you
can rebase your branch.

Description

This will add a few more options to configure JDA.

Also added an option to set the okhttp client used by JDA to allow using a single okhttp client for all JDA sessions Additionally I also fixed the documentation in a few locations

Also ensure the token in DefaultShardManager is only validated once

MinnDevelopment · 2018-07-06T12:42:40Z

src/main/java/net/dv8tion/jda/core/entities/impl/JDAImpl.java

        }

        userResponse = checkToken(login);
+        requester.shutdownNow();


Maybe we should call JDA#shutdownNow here

MinnDevelopment · 2018-07-06T12:58:07Z

src/main/java/net/dv8tion/jda/bot/sharding/DefaultShardManagerBuilder.java

@@ -535,7 +560,7 @@ public DefaultShardManagerBuilder setIdleProvider(final IntFunction<Boolean> idl
     * @throws IllegalArgumentException
     *         if the provided OnlineStatus is null or {@link net.dv8tion.jda.core.OnlineStatus#UNKNOWN UNKNOWN}
     *
-     * @return The {@link net.dv8tion.jda.bot.sharding.DefaultShardManagerBuilder DefaultShardManagerBuilder} instance. Useful for chaining.
+     * @return @return The DefaultShardManagerBuilder instance. Useful for chaining.


buildAsync -> build() buildBlocking -> build().awaitReady()

Now the methods accepting threadpools additionally accept an optional boolean to enable/disable automatic shutdown Additionally I added generic bounds to the providers (where needed)

MinnDevelopment · 2018-07-31T17:11:22Z

Example usage:

JDABuilder builder = new JDABuilder(AccountType.BOT).setToken(BOT_TOKEN);
builder.setCorePoolSize(1);
builder.setCallbackPool(new ForkJoinPool(), true);
builder.addEventListener(new CommandHandler());
JDA api = builder.build();
api.awaitStatus(JDA.Status.AWAITING_LOGIN_CONFIRMATION);
log.info("Shard has started setting up");

kantenkugel

Some renaming / improvements i would make. Nothing major

kantenkugel · 2018-08-02T10:27:25Z

README.md

 ## Creating the JDA Object
 Creating the JDA Object is done via the JDABuilder class by providing an AccountType (Bot/Client).
 After setting the token via setter,
-the JDA Object is then created by calling the `.buildBlocking()` or the `.buildAsync()` (non-blocking login) method.
+the JDA Object is then created by calling the `.build()` (non-blocking login) method.
+When `build()` returns JDA might not have finished starting up, however you can use `awaitReady()`


Might change punctuation to "When build() returns**,** JDA might not have finished starting up**.** However**,** ..."

README.md


 **Example**:

 ```java
-JDA jda = new JDABuilder(AccountType.BOT).setToken("token").buildBlocking();
+JDA jda = new JDABuilder(AccountType.BOT).setToken("token").build();


kantenkugel · 2018-08-02T10:31:45Z

README.md

@@ -67,7 +79,7 @@ public class MessageListener extends ListenerAdapter
    public static void main(String[] args)
            throws LoginException, RateLimitedException, InterruptedException


If i understand correctly, build() on its own will not throw an InterruptedException anymore, right?

kantenkugel · 2018-08-02T11:42:31Z

src/main/java/net/dv8tion/jda/bot/sharding/ThreadPoolProvider.java

+{
+    T provide(int shardId);
+
+    default boolean isAutomaticShutdown(int shardId)


I would add javadocs here, and possibly rename to shouldShutdownAutomatically cuz isAutomaticShutdown sounds like JDA asks the ThreadPoolProvider if the pool already shuts down automatically rather than asking if JDA should shut it down

kantenkugel · 2018-08-02T11:45:53Z

src/main/java/net/dv8tion/jda/core/JDABuilder.java

     */
+    @Deprecated
    public JDA buildBlocking(JDA.Status status) throws LoginException, InterruptedException
    {
        Checks.notNull(status, "Status");
        Checks.check(status.isInit(), "Cannot await the status %s as it is not part of the login cycle!", status);
        JDA jda = buildAsync();


Would use build() here instead of buildAsync(). The checks are also not needed anymore, as they are present in jda#awaitStatus()

They are needed because we don't want to build if they fail

Ah right... makes sense

kantenkugel · 2018-08-02T11:46:44Z

src/main/java/net/dv8tion/jda/core/JDABuilder.java

     */
+    @Deprecated
    public JDA buildBlocking() throws LoginException, InterruptedException
    {
        return buildBlocking(Status.CONNECTED);


Maybe switch to build().awaitReady()? Its not important here tho

Add option for setting the core pool size for rate-limit handling

580e967

Also added an option to set the okhttp client used by JDA to allow using a single okhttp client for all JDA sessions Additionally I also fixed the documentation in a few locations

MinnDevelopment added type: feature version: minor labels Jul 5, 2018

MinnDevelopment changed the title ~~Add option for setting the core pool size for rate-limit handling~~ Add more configurations for JDABuilder Jul 5, 2018

Add option for setting rate-limit thread-pool through builders

0576639

Also ensure the token in DefaultShardManager is only validated once

MinnDevelopment commented Jul 6, 2018

View reviewed changes

Shutdown JDA when token verification fails

6034d18

MinnDevelopment commented Jul 6, 2018

View reviewed changes

Cleanup missed documentation issues

1d01a68

MinnDevelopment requested a review from DV8FromTheWorld July 8, 2018 12:04

MinnDevelopment added the status: completed has been completed but is not yet released label Jul 8, 2018

MinnDevelopment modified the milestones: Release 3.7.1, Upcoming Release Jul 11, 2018

MinnDevelopment mentioned this pull request Jul 21, 2018

Improve use of MDC #721

Merged

2 tasks

Merge rate-limit and callback pools into single pool

fd061ac

This comment has been minimized.

Sign in to view

MinnDevelopment added 5 commits July 29, 2018 12:21

Change build methods in JDABuilder

ac24672

buildAsync -> build() buildBlocking -> build().awaitReady()

Improve error message for shutdown

c5b8f38

Use ForkJoinPool for callbacks by default

ed9d63b

Improve configuration methods

00bef5a

Now the methods accepting threadpools additionally accept an optional boolean to enable/disable automatic shutdown Additionally I added generic bounds to the providers (where needed)

Shutdown pools by default

03484dd

MinnDevelopment added 5 commits July 31, 2018 19:13

Fix javadoc errors

6d14a09

Improve getGuildById performance

16702d2

Add nested implementation for provider

d9bcb34

Update the example for new builder methods

dc82e6b

Add note about userbots and selfbots

b24b164

kantenkugel requested changes Aug 2, 2018

View reviewed changes

Adjustments

0ff348b

Remove remaining references to old build methods

17909c0

kantenkugel approved these changes Aug 2, 2018

View reviewed changes

MinnDevelopment added 4 commits August 2, 2018 20:35

Update README

3ff223f

Update warning message for WebSocket RateLimit

f2e57af

Update README to mention other options in introduction

370edc5

Cleanup warning message

e6c9298

MinnDevelopment merged commit 4cc724e into development Aug 3, 2018

MinnDevelopment modified the milestones: Upcoming Release, 3.8.0 Aug 5, 2018

MinnDevelopment deleted the feature/configurations branch August 29, 2018 12:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add more configurations for JDABuilder #705

Add more configurations for JDABuilder #705

MinnDevelopment commented Jul 5, 2018 •

edited

Loading

MinnDevelopment Jul 6, 2018

MinnDevelopment Jul 6, 2018

This comment has been minimized.

MinnDevelopment commented Jul 31, 2018

kantenkugel left a comment

kantenkugel Aug 2, 2018 •

edited

Loading

This comment was marked as resolved.

kantenkugel Aug 2, 2018

kantenkugel Aug 2, 2018

kantenkugel Aug 2, 2018

MinnDevelopment Aug 2, 2018

kantenkugel Aug 2, 2018

kantenkugel Aug 2, 2018

		@@ -67,7 +79,7 @@ public class MessageListener extends ListenerAdapter
		public static void main(String[] args)
		throws LoginException, RateLimitedException, InterruptedException

Add more configurations for JDABuilder #705

Add more configurations for JDABuilder #705

Conversation

MinnDevelopment commented Jul 5, 2018 • edited Loading

Pull Request Etiquette

Description

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

MinnDevelopment commented Jul 31, 2018

kantenkugel left a comment

Choose a reason for hiding this comment

kantenkugel Aug 2, 2018 • edited Loading

Choose a reason for hiding this comment

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MinnDevelopment commented Jul 5, 2018 •

edited

Loading

kantenkugel Aug 2, 2018 •

edited

Loading