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

fix: KeyError when using method 'individual' #1925

Merged
merged 10 commits into from May 13, 2023

Conversation

doluk
Copy link
Contributor

@doluk doluk commented Feb 15, 2023

Summary

Resolve the origin of the variable cmd in ApplicationCommandMixin.register_commands() and fix a bug in the same method when using the sync method individual.

Information

  • This PR fixes an issue.
  • This PR adds something new (e.g. new method or parameters).
  • This PR is a breaking change (e.g. methods or parameters removed/renamed).
  • This PR is not a code change (e.g. documentation, README, typehinting,
    examples, ...).

Checklist

  • I have searched the open pull requests for duplicates.
  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
  • If type: ignore comments were used, a comment is also left explaining why.

The variable cmd originates from the for-loop in line 602, if the method is not "bulk". Therefore the old code throws a KeyError-Exception, because the key name doesnt't exists. cmd only has the keys action, command and id. This breaks the method individual for syncing commands.

The variable cmd originates from the for-loop in line 602, if the method is not "bulk". Therefore the old code throws a KeyError Exception because the key `name` doesnt't exists. This breaks the method `individual` for syncing commands.

Signed-off-by: Lukas Dobler <69309597+doluk@users.noreply.github.com>
@doluk doluk requested a review from a team as a code owner February 15, 2023 12:17
@doluk doluk changed the title Fix KeyError when using method 'individual' Fix: KeyError when using method 'individual' Feb 15, 2023
@Lulalaby Lulalaby changed the title Fix: KeyError when using method 'individual' fix: KeyError when using method 'individual' Feb 15, 2023
@codecov
Copy link

codecov bot commented Feb 15, 2023

Codecov Report

Merging #1925 (3966f1f) into master (a20a6e8) will not change coverage.
The diff coverage is 0.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1925   +/-   ##
=======================================
  Coverage   33.22%   33.22%           
=======================================
  Files          97       97           
  Lines       19092    19092           
=======================================
  Hits         6343     6343           
  Misses      12749    12749           
Flag Coverage Δ
macos-latest-3.10 33.20% <0.00%> (ø)
macos-latest-3.11 33.20% <0.00%> (ø)
macos-latest-3.8 33.21% <0.00%> (ø)
macos-latest-3.9 33.21% <0.00%> (ø)
ubuntu-latest-3.10 33.20% <0.00%> (ø)
ubuntu-latest-3.11 33.20% <0.00%> (ø)
ubuntu-latest-3.8 33.21% <0.00%> (ø)
ubuntu-latest-3.9 33.21% <0.00%> (ø)
windows-latest-3.10 33.20% <0.00%> (ø)
windows-latest-3.11 33.20% <0.00%> (ø)
windows-latest-3.8 33.21% <0.00%> (ø)
windows-latest-3.9 33.21% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
discord/bot.py 18.45% <0.00%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a20a6e8...3966f1f. Read the comment docs.

@BobDotCom
Copy link
Member

Can you please provide an example of code that would cause the KeyError previously?

@doluk
Copy link
Contributor Author

doluk commented Feb 17, 2023

Yes, I will post an example in a bit. Here is the non-code version:

  • Create a discord client with slash commands
  • Then execute client.sync_commands(method='individual')

Signed-off-by: Lukas Dobler <69309597+doluk@users.noreply.github.com>
@JustaSqu1d JustaSqu1d added good first issue Good for newcomers priority: medium Medium Priority status: awaiting review Awaiting review from a maintainer python labels Feb 22, 2023
@plun1331 plun1331 added the bug Something isn't working label Feb 26, 2023
@Middledot
Copy link
Member

I think it would be better if the name of the command was passed into register() instead of keeping the cmd variable "floating" like that. Also, you should add a changelog entry

@Lulalaby
Copy link
Member

Lulalaby commented May 8, 2023

@doluk please work on that, we want to get it finished <3

@doluk
Copy link
Contributor Author

doluk commented May 12, 2023

@Lulalaby @Middledot
I removed the floating variables now completely and added the changelog entry as requested.
Sorry for the delay. Life being life....

@Lulalaby
Copy link
Member

We know, all fine

Copy link
Member

@Middledot Middledot left a comment

Choose a reason for hiding this comment

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

Lgtm

@Lulalaby Lulalaby merged commit 3babb79 into Pycord-Development:master May 13, 2023
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers priority: medium Medium Priority status: awaiting review Awaiting review from a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants