-
-
Notifications
You must be signed in to change notification settings - Fork 24
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
Getting "RuntimeError: GitHub client has already been collected." error while using API from a Task #109
Comments
It gives the same error if invoked as a background task in FastAPI as well: @app.post("/github/webhook")
async def github_webhook(request: Request, background_tasks: BackgroundTasks):
...
if isinstance(event, WebhookPullRequestReopened):
background_tasks.add_task(
pull_request_opened,
event.installation.id,
event.repository.owner.login,
event.repository.name,
event.pull_request.id,
{},
) |
Found the solution. It works with with github.installation_github(installation_id) as installation_github:
pr = (
await installation_github.rest.pulls.async_get(
owner, repo, pull_request_number
)
).parsed_data
logger.info(f"Pull request title: {pr.title}") But could you please explain why? |
The error occurs when the github client instance has been collected by the gc. I'm not sure why the instance is collected in your code, could you please try something like this: def installation_github(installation_id: int):
return GitHub(
AppInstallationAuthStrategy(
config.GitHubAppID,
config.GithubAppPrivateKey.replace("\\n", "\n"),
installation_id,
)
)
async def pull_request_opened(
installation_id: int,
owner: str,
repo: str,
pull_request_id: int,
settings: dict[str, str],
):
logger.info(
f"Start: installation_id={installation_id}, pull_request_id={pull_request_id}"
)
# keep a reference to the instance
g = installation_github(installation_id)
pr = (
await g.rest.pulls.async_get(
owner, repo, pull_request_id
)
).parsed_data
logger.info(f"Pull request title: {pr.title}") and which python version you are using? |
I reproduce the error with a minimal example. Now i'm sure that this error is caused by not holding a reference to the Following is the minimal example: import weakref
class A:
def __init__(self) -> None:
weakref.finalize(self, lambda: print("finalize"))
@property
def b(self) -> "B":
return B(self)
class B:
def __init__(self, a: A) -> None:
self._a = weakref.ref(a)
@property
def a(self) -> A:
if a := self._a():
return a
raise RuntimeError("a is dead")
def func() -> A:
return A()
# This will cause error
# func return value is collected immediately after getting b
func().b.a
# This is ok
a = func()
a.b.a |
Yep, this works: g = installation_github(installation_id)
return (await g.rest.pulls.async_get(owner, repo, pull_request_number)).parsed_data Thanks for checking on this. You would probably need to mention that behaviour in the readme to help others. Still enjoying the library - probably the best GitHub API client for Python with async support! |
I have the following module:
While trying to call
pull_request_opened()
in a Task:I'm getting error:
The text was updated successfully, but these errors were encountered: