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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(enf_parts): remove leading/trailing whitespace from parts parameter values #176

Merged
merged 1 commit into from
Jun 13, 2024

Conversation

pidi3000
Copy link
Contributor

@pidi3000 pidi3000 commented May 21, 2024

When making a request you can specify the resource parts you want
for example:

cli.playlistItems.list(
    playlist_id=id,
    parts="snippet,contentDetails"
)

But adding a whitespace between values

cli.playlistItems.list(
    playlist_id=id,
    parts="snippet, contentDetails" # <- note the whitespace
)

results in the error message:
Parts contentDetails for resource playlistItems not support
(took me a couple of minutes to notice the extra space 馃槅)

By running strip() on each part, leading/trailing whitespaces are ignored.
This also works for list/set/tuple
ie:

parts=("snippet"," contentDetails")
parts=["snippet "," contentDetails"]

@pidi3000
Copy link
Contributor Author

Another consideration would be to make the error message a bit clearer.

Instead of doing
not_support_parts = ",".join(parts.difference(support_parts))
resulting in: Parts contentDetails for resource playlistItems not support

use
not_support_parts = ", ".join(f"'{part}'" for part in parts.difference(support_parts))
resulting in: Parts ' contentDetails' for resource playlistItems not support
(note the extra quotes around the part string)

This would make spotting errors like that a bit easier in the future

@MerleLiuKun
Copy link
Member

LGTM!

@MerleLiuKun MerleLiuKun merged commit 280d807 into sns-sdks:master Jun 13, 2024
6 of 8 checks passed
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.

None yet

2 participants