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

Documentation of using AsyncGitHubAPI/SyncGithubAPI incorrect #47

Open
joshuadavidthomas opened this issue Jan 9, 2025 · 1 comment
Labels
bug Something isn't working documentation Improvements or additions to documentation

Comments

@joshuadavidthomas
Copy link
Owner

joshuadavidthomas commented Jan 9, 2025

In the README when showing using the clients on their own, we are missing the requester argument.

from django_github_app.github import AsyncGitHubAPI
from django_github_app.models import Installation

# Access public endpoints without authentication
async def get_public_repo():
    async with AsyncGitHubAPI() as gh:
        return await gh.getitem("/repos/django/django")


# Interact as the GitHub App installation
async def create_comment(repo_full_name: str):
    # Get the installation for the repository
    installation = await Installation.objects.aget(
        repositories__full_name=repo_full_name
    )

    async with AsyncGitHubAPI(installation_id=installation.installation_id) as gh:
        await gh.post(
            f"/repos/{repo_full_name}/issues/1/comments", data={"body": "Hello!"}
        )

    # You can either provide the `installation_id` as above, or the `Installation` instance
    # itself
    async with AsyncGitHubAPI(installation=installation) as gh:
        await gh.post(
            f"/repos/{repo_full_name}/issues/1/comments", data={"body": "World!"}
        )

In both cases, we are just providing a thin layer over gidgethub.abc.GitHubAPI with support for the Installation model and an opinionated use of httpx as the HTTP client. So we still need to provide all the arguments that the abstract base GitHubAPI expects, namely requester: str as the first argument to the class.

Corrected documentation example:

from django_github_app.github import AsyncGitHubAPI
from django_github_app.models import Installation

# Access public endpoints without authentication
async def get_public_repo():
    async with AsyncGitHubAPI("example-github-app") as gh:
        return await gh.getitem("/repos/django/django")

In the webhook view, we grab it from the app_settings.SLUG so it's not a problem there.

Either we can update the documentation to indicate this or mirror the webhook view and use app_settings.SLUG. Or, maybe even better than that, allow for providing a custom requester and if one is not provided, fallback to the app_settings.SLUG.

@joshuadavidthomas joshuadavidthomas added bug Something isn't working documentation Improvements or additions to documentation labels Jan 9, 2025
@joshuadavidthomas
Copy link
Owner Author

Regardless of if we don't add either of the app_setting.SLUG options, I do think the documentation needs to be updated at a minimum.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

1 participant