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 OAUTH_CUSTOM_FULL mode #527

Merged
merged 2 commits into from
Jan 21, 2024
Merged

Fix OAUTH_CUSTOM_FULL mode #527

merged 2 commits into from
Jan 21, 2024

Conversation

MarvinSchenkel
Copy link
Contributor

This PR adds the missing AuthType.OAUTH_CUSTOM_FULL mode when calculating the headers. We (Music Assistant) use our own web-based OAuth process to retrieve an acces token. In this scenario we also provide the other required headers to interact with Youtube Music.

Copy link
Owner

@sigma67 sigma67 left a comment

Choose a reason for hiding this comment

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

Shouldn't this already be supported via

https://github.com/sigma67/ytmusicapi/blob/main/ytmusicapi/ytmusic.py#L190-L191

If not, can we have a test as well that covers the difference and ensures it won't break again in the future?

@MarvinSchenkel
Copy link
Contributor Author

Good point! In that case I would need to make some changes to this line, as the token is None for full custom oauth:

self._headers["authorization"] = self._token.as_auth()
. This currently throws an error.

@sigma67
Copy link
Owner

sigma67 commented Jan 21, 2024

Yeah, is it enough to just not set authorization if self.auth_type == AuthType.OAUTH_CUSTOM_FULL: ?

@MarvinSchenkel
Copy link
Contributor Author

Yeah, is it enough to just not set authorization if self.auth_type == AuthType.OAUTH_CUSTOM_FULL: ?

Absolutely, fixed that

@sigma67 sigma67 merged commit 294fb81 into sigma67:master Jan 21, 2024
2 of 3 checks passed
@sigma67
Copy link
Owner

sigma67 commented Jan 21, 2024

@MarvinSchenkel please submit to main next time, master is no longer used

@MarvinSchenkel
Copy link
Contributor Author

@MarvinSchenkel please submit to main next time, master is no longer used

Sorry I wasn't aware. My existing fork was still pointing to master.

@sigma67
Copy link
Owner

sigma67 commented Jan 21, 2024

I deleted the master branch in this repo so this can't happen anymore 👍

(don't worry I'll push your commit shortly)

sigma67 pushed a commit that referenced this pull request Jan 21, 2024
* Fix OAUTH_CUSTOM_FULL mode

* Fix OAUTH_CUSTOM_FULL mode
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