Skip to content

Conversation

@mAxYoLo01
Copy link
Contributor

About

image
Previously, the library did not account for bots which were present in more than 200 guilds BUT used guild commands. Now, the function when syncing uses a paginator to make sure all guilds are acquired.

Checklist

  • The pre-commit code linter has been run over all edited files to ensure the code is linted.
  • I've ensured the change(s) work on 3.8.6 and higher.

I've made this pull request: (check all that apply)

  • For the documentation
  • To add a new feature
  • As a general enhancement
  • As a refactor of the library/the library's code
  • To fix an existing bug
  • To resolve #ISSUENUMBER

This is:

  • A breaking change

@mAxYoLo01 mAxYoLo01 requested a review from EepyElvyra December 2, 2022 00:53
Copy link
Member

@AstreaTSS AstreaTSS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going through every single guild is a very slow operation. Perhaps a warning would be best? Maybe advise the user to not do this if more than one iteration is needed?

Also, while this doesn't relate to this PR, maybe adding in a toggle to disable guild-based syncing would be a good feature too, as many only need to update global commands.

@mAxYoLo01 mAxYoLo01 changed the title feat: Add Guild.get_all_guilds() + paginate guilds when syncing feat: Paginate guilds when syncing Dec 2, 2022
@mAxYoLo01
Copy link
Contributor Author

Ok, I have modified the PR to make the function private to Client, as we already have bot.guilds and it would act as a duplicate. Also allows to reduce the time taken as it's not converted to a Guild object

@mAxYoLo01
Copy link
Contributor Author

PR was tested and it resolves the issue that one person had

@EepyElvyra EepyElvyra merged commit 8543428 into interactions-py:unstable Dec 2, 2022
@mAxYoLo01 mAxYoLo01 deleted the get_guilds_fix branch December 2, 2022 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants