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

Add Accept headers to grant retrieval #57

Closed
wants to merge 1 commit into from

Conversation

Kludex
Copy link

@Kludex Kludex commented Dec 4, 2022

The code is already assuming the response will be a json, but that's necessarily true.

I have a case, that by default they don't send a JSON... 馃槄

The case is for WakaTime.

@sonarcloud
Copy link

sonarcloud bot commented Dec 4, 2022

Kudos, SonarCloud Quality Gate passed!聽 聽 Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@Colin-b Colin-b self-requested a review December 4, 2022 19:22
Copy link
Owner

@Colin-b Colin-b left a comment

Choose a reason for hiding this comment

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

Thanks for the bugfix. It should indeed be fine like this. Do you mind ensuring in the test suite that this header is sent for each auth this affects? If not I'll do it when I find some time.

Could you please also tell me what kind of data was sent? As it would be even better to handle this specific case as well instead of requiring JSON.

@Kludex
Copy link
Author

Kludex commented Dec 4, 2022

If not I'll do it when I find some time.

You can do it when you find some time. Thanks! 馃檹

I'm already using this branch.

Could you please also tell me what kind of data was sent? As it would be even better to handle this specific case as well instead of requiring JSON.

You mean to support multiple formats?

Anyway, the code you can use:

import hashlib
import os

import httpx
from httpx_auth import OAuth2AuthorizationCode
from pydantic import BaseSettings


class Settings(BaseSettings):
    wakatime_client_id: str
    wakatime_client_secret: str


settings = Settings()

auth = OAuth2AuthorizationCode(
    authorization_url="https://wakatime.com/oauth/authorize",
    token_url="https://wakatime.com/oauth/token",
    client_id=settings.wakatime_client_id,
    client_secret=settings.wakatime_client_secret,
    redirect_uri_endpoint="oauth2callback",
    redirect_uri_port=8000,
    scope="email,read_stats,read_logged_time",
    state=hashlib.sha1(os.urandom(40)).hexdigest(),
)
client = httpx.Client(base_url="https://wakatime.com/api/v1/", auth=auth)
res = client.get("/users/current")
print(res.json())

@Kludex
Copy link
Author

Kludex commented Dec 4, 2022

You can verify the Content-Type instead of restricting the Accept. 馃憤

@Colin-b
Copy link
Owner

Colin-b commented Dec 4, 2022

You can verify the Content-Type instead of restricting the Accept. 馃憤

Yes that's exactly my point, I'll see if I can add a WakaTimeAuth as well

@Colin-b
Copy link
Owner

Colin-b commented Dec 4, 2022

I just created an app on WakaTime and I'm able to make it work without requesting JSON, I pushed it to bugfix/html_content_type in the meantime, it is still in draft as I need to make sure we can handle error responses properly as well and add some more coverage on this new feature. I'll also add WakaTimeAuth (and documentation on how to set it up) at some point to ease up the setup. Even if the solution to your problem is different, I'll make sure to include you in the changelog :) Thanks

@Colin-b Colin-b closed this Dec 4, 2022
@Colin-b Colin-b mentioned this pull request Dec 4, 2022
@Kludex
Copy link
Author

Kludex commented Dec 5, 2022

Why adding the class itself instead of just verifying the content type, and reading the content properly? 馃

@Colin-b
Copy link
Owner

Colin-b commented Dec 5, 2022

Both will be done, the class is to ease the setup for wakatime users (no more auth url required and handling of the comma separated scopes when providing a list of scopes). This will prevent the need to read the authentication part of the setup, only focusing on actually accessing data :)

@Colin-b
Copy link
Owner

Colin-b commented Apr 25, 2023

Hello @Kludex ,

Version 0.16.0 is now available on pypi with this feature. I advise you to use the new WakaTimeAuthorizationCode authentication class to ease the readability of your code.

Best Regards

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