Skip to content

Implement PUT requests in auto_register #80

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

Merged
merged 4 commits into from
Jan 27, 2021

Conversation

quirky-bluejay
Copy link
Contributor

@quirky-bluejay quirky-bluejay commented Jan 27, 2021

About this pull request

This implements PUT requests for registering commands when auto_register and auto_delete are both true.
This will reduce the amount of requests made to discord, only one per guild/global compared to one request for each command for each scope.

Changes

  • Added a to_dict to SlashCommand function to get all commands into a dictionary, this reduces duplicated code from register_all_commands and sync_all_commands
  • Added sync_all_commands to SlashCommand which uses PUT requests to update the commands.
  • Automatically call sync_all_commands when both auto_register and auto_delete are True
  • Only call register_all_commands when auto_register is True and auto_delete isn't.
  • Only call delete_unused_commands when auto_register is True and auto_delete isn't.
  • Add put_slash_commands to http

Checklist

  • I checked this pull request runs on Python 3.6.X.
  • This fixes something in Issues.
    • Issue:
  • This adds something new.
  • There is/are breaking change(s).
  • (If required) Document string is updated/added.
  • This changes anything except python code. (README, docs, etc.)

@eunwoo1104 eunwoo1104 self-requested a review January 27, 2021 08:23
@eunwoo1104 eunwoo1104 self-assigned this Jan 27, 2021
Copy link
Contributor

@eunwoo1104 eunwoo1104 left a comment

Choose a reason for hiding this comment

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

It looks like to_dict has wait_until_ready, but register_all_commands hasn't. This can be a problem if user just want to get a dict of the commands but can't use the coroutine. So we should also change the position of wait_until_ready.

@quirky-bluejay
Copy link
Contributor Author

quirky-bluejay commented Jan 27, 2021

I decided to move wait_until_ready to to_dict because i thought that someone using it would want to make sure that all the commands are loaded.
If you think that the pros of not being an asyncronous functions outweigh that, i'll change it back.

@eunwoo1104
Copy link
Contributor

I decided to move wait_until_ready to to_dict because i thought that someone using it would want to make sure that all the commands are loaded.
If you think that the pros of not being an asyncronous functions outweigh that, i'll change it back.

Ah, yeah you're right. I didn't think of that. I'll then accept this pull request, since anything else seems fine.

@eunwoo1104
Copy link
Contributor

Merging this pull request, thank you always for contributing!

@eunwoo1104 eunwoo1104 merged commit cbe4ab2 into interactions-py:master Jan 27, 2021
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.

2 participants