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

[bug] Pydantic model definition errors for fragments with nesting #253

Closed
fekstr opened this issue Dec 15, 2023 · 3 comments
Closed

[bug] Pydantic model definition errors for fragments with nesting #253

fekstr opened this issue Dec 15, 2023 · 3 comments

Comments

@fekstr
Copy link

fekstr commented Dec 15, 2023

Description

Pydantic types are incorrectly defined when fragments contain nested nodes.

Example:

# works
fragment IssueFields on Issue {
    id
}

query getIssue($id: ID!) {
    node(id: $id) {
        ...IssueFields
    }
}

# works
query getIssue($id: ID!) {
    node(id: $id) {
        ...on Issue {
            id
            author {
                login
            }
        }
    }
}

# does not work
fragment IssueFields on Issue {
    id
    author {
        login
    }
}

query getIssue($id: ID!) {
    node(id: $id) {
        ...IssueFields
    }
}

There are two issues in the last case.

  1. The generated IssueFieldsAuthor type includes a required __typename field that is not included in the generated query, causing a validation error.
  2. The generated IssueFields type contains an author field of type Optional["IssueFieldsAuthor"] that somehow seems to be undefined at instantiation, yielding the following error:
pydantic.errors.PydanticUserError: `GetIssue` is not fully defined; you should define `GetIssueNodeNode`, then call `GetIssue.model_rebuild()`

The first issue disappears when including __typename in the generated query (in the client get_issue method).

The second issue disappears when defining IssueFieldsAuthor before IssueFields in fragments.py.

Reproduction

  1. Define these queries
# queries.graphql

fragment IssueFields on Issue {
    id
    author {
        login
    }
}

query getIssue($id: ID!) {
    node(id: $id) {
        ...IssueFields
    }
}

Use the GitHub schema.

Generate the types with ariadne-codegen.

Run the following:

import asyncio

# use the path to the generated client here
from src.graphql.graphql_client import Client


async def run():
    access_token = "<YOUR TOKEN>"
    client = Client(
        url="https://api.github.com/graphql",
        headers={
            "Authorization": f"Bearer {access_token}",
            "Content-Type": "application/json",
            "Accept": "application/vnd.github.bane-preview+json",
        },
    )

    issue = await client.get_issue(id="I_kwDOIPDwls55yyZU")

    print(issue)


if __name__ == "__main__":
    asyncio.run(run())

Environment

Python 3.11.6

@mat-sop
Copy link
Contributor

mat-sop commented Dec 15, 2023

Hey, I managed to reproduce both of these issues:

  1. It Looks like we omit artificially added __typename in all fragment strings.
  2. In the latest release we removed all model_rebuild calls. I tested this change with fragments, but it seems that the issue only occurs for nested fragments. We need to restore some of the removed model_rebuild calls.

I will create a separate issue with a more accurate description for the 1. issue and let's keep the current one for 2.

@rafalp
Copy link
Contributor

rafalp commented Jan 25, 2024

Fixed in #267

__typename is tracked in #254

@rafalp rafalp closed this as completed Jan 25, 2024
@nick4fake
Copy link

nick4fake commented Jul 15, 2024

I am sorry, so is there a solution for this issue?

Edit: whoops, I am sorry, used 0.11 instead of 0.13

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 a pull request may close this issue.

4 participants