-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
refactor: Update get_github_installation_ids to use httpx #6451
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good change! Why not create a generic async fetch from now, too?
E.g.:
async def async_fetch(
method: str = 'GET',
url: str = '',
headers: dict = None,
params: dict = None,
):
try:
async with httpx.AsyncClient() as client:
response = await client.request(
method=method.upper(),
url=url,
headers=headers,
params=params,
)
response.raise_for_status()
return response.json()
except httpx.HTTPError as e:
# Handle HTTP errors
raise Exception(f'HTTP error: {e}')
except Exception as e:
# Handle other exceptions
raise Exception(f'Error: {e}')
I like this idea - I'll do it in a later PR when there's time. |
…AI#6451) Co-authored-by: openhands <[email protected]>
…AI#6451) Co-authored-by: openhands <[email protected]>
This PR updates the
get_github_installation_ids
function to usehttpx
instead ofrequests
withcall_sync_from_async
. This change makes the code more consistent with other functions in the file and more efficient by using native async HTTP requests.Changes:
requests
withhttpx.AsyncClient()
httpx.HTTPError
response.close()
as it is handled by the context managerTo run this PR locally, use the following command: