Closed dentatar closed 4 months ago
+1
Have the same exception
+1
Have the same exception
I have the same issue. Downgrading to 0.3.1 fixed it, so I guess something in 0.4.0 broke it.
Thanks for the report! I'll try to look into it this weekend. Has anyone gathered some more detail of what could have caused the issue? That would really help me dive straight into the fixing 🙂
I've investigated further. The issue is this change, specifically L289. The node type names are now passed as a ForwardRef
to the generic. This ref is not resolved correctly.
https://github.com/strawberry-graphql/strawberry-sqlalchemy/blob/4fa950fac41b56cb48b82d78a69b625f3f92afd3/src/strawberry_sqlalchemy_mapper/mapper.py#L278-L293
Since relay.Edge
is generic, we save the value of the TypeVar for the node type inside of StrawberryField.specialized_type_var_map
to later replace it in StrawberryField.resolve_type
:
The problem in this case: the mapped type is not a StrawberryType
, but a ForwardRef equaling the type name we expect it to be. This cannot be resolved, because the generic resolver does not convert the ForwardRef into a StrawberryAnnotation to call evaluate on it.
I did this as a test, but this yielded another error: the type Employee
is not defined in the scope. This is because we have no strawberry.lazy info about the actual location of the type Employee
because we just expect that it will exist under this name later.
I'm still looking for a quick and easy fix, but long-term I see this being fixed through one of these options:
For me personally, a central registry is the preferred approach since it gives full control over the way we access types. This is a mid-term effort and requires heavy refactoring, so having something that works short-term to fix the occurring issues would be preferable.
/cc @mattalbr
Adopt strawberry-django's behavior /cc @bellini666 how are you implementing lazy refs in relationships?
Actually in strawberry django all relations and the types to those relations should be created explicitly: https://strawberry-graphql.github.io/strawberry-graphql-django/guide/fields/#relationships , and thus, those can be done using Lazy types
If we try to map a relationship using auto or passing to the fields list, we map them to a type that only has a pk attribute (https://github.com/strawberry-graphql/strawberry-graphql-django/blob/main/strawberry_django/fields/types.py#L250) or to the Node interface when using relay. But we usually suggest users to map relations explicitly
Having said that, based on how this code works, a type registry is probably the way to go for this.
TBH, I'm not totally familiar with this code but doesn't something like that already exists when creating the Edge/Connection? i.e. relay.Edge[self.mapped_types[type_name]]
Also, it seems that I'm the one who caused this issue so I can try to fix it as well :)
This should fix the issue: https://github.com/strawberry-graphql/strawberry-sqlalchemy/pull/106
I've merged #106 to ensure a quick fix for the affected projects. We can keep this issue open until we are certain about the definitive future solution for this. The use of lazy types is exactly what I've envisioned, but I'm looking forward to hear matt's opinion on the matter.
@dentatar @Gianfranco753 @0x587 @johansigfrids please try out the new release 0.4.1
and let us know if you encounter any additional issues. Thanks for reporting the bug! 🙂
I'm trying to catch up here, but you two (Erik and Thiago) understand strawberry's typing system WAY better than me. I really need to hop in and play around with Thiago's new relay code to get a better understanding. That said, I took a look at #106 and it looks good to me. I appreciate you both!
Well, I finally had time to test it out and it is not quite working yet. The ForwardRef error is gone and it starts up fine, but attempting to query the connection results in the following error:
EmployeeEdge.__init__() missing 1 required keyword-only argument: 'cursor'
GraphQL request:4:5
3 | name
4 | employees {
| ^
5 | edges {
Traceback (most recent call last):
File "/Users/johansigfrids/mapper_test/venv/lib/python3.10/site-packages/graphql/execution/execute.py", line 528, in await_result
return_type, field_nodes, info, path, await result
File "/Users/johansigfrids/mapper_test/venv/lib/python3.10/site-packages/strawberry/schema/schema_converter.py", line 675, in _async_resolver
return await await_maybe(
File "/Users/johansigfrids/mapper_test/venv/lib/python3.10/site-packages/strawberry/utils/await_maybe.py", line 12, in await_maybe
return await value
File "/Users/johansigfrids/mapper_test/venv/lib/python3.10/site-packages/strawberry_sqlalchemy_mapper/mapper.py", line 467, in wrapper
edges=[
File "/Users/johansigfrids/mapper_test/venv/lib/python3.10/site-packages/strawberry_sqlalchemy_mapper/mapper.py", line 468, in <listcomp>
edge_type(
TypeError: EmployeeEdge.__init__() missing 1 required keyword-only argument: 'cursor'
Got the same issues in my project. A one-to-many relationship not working with missing 1 required keyword-only argument: 'cursor'
.
My simple Schema:
class A(Base):
__tablename__ = "a"
id: Mapped[str] = mapped_column(primary_key=True, index=True)
bs: Mapped[List["B"]] = relationship(back_populates="a")
class B(Base):
__tablename__ = "b"
id: Mapped[str] = mapped_column(primary_key=True, index=True)
a_id: Mapped[str] = mapped_column(ForeignKey("a.id"), nullable=False)
a: Mapped[A] = relationship(back_populates="bs")
Downgrading from 0.4.2 to 0.3.1 fixes relationships for the time being. I don't know the project enough yet to help you on this.
Ok, here is what is going on here:
When trying to instantiate the connection and the edges in here we are missing some attributes:
edge_type
is a subclass of strawberry's relay.Edge
, meaning it requires a node
and a cursor
(we are missing the cursor)connection_type
is a subclass of strawberry's relay.Connection
, meaning it requires a list of edges
and a page_info
(we are missing the page_info)This is because in my relay PR I changed the Edge
we create to use relay.Edge
as its base and I also changed the Connection
we create to use relay.Connection
as its base
Because we didn't have any tests for the resolvers themselves I didn't even see that they were missing those.
We have 2 options to solve that:
1) Remove the bases
from those places so that the auto generated edges/connections will not require cursor
/page_info
. This is the easiest way to fix this but it would also make automatic connections a lot different from explicitly defined connections.
2) Add that missing info, but where should we retrieve that from? From what I can tell the dataloader approach is not actually using any pagination information (i.e. it will just load everything from the database), meaning adding a cursor to it is mostly useless (unless we change how it works)
@mattalbr @erikwrede thoughts about this?
Got the same issues in my project. A one-to-many relationship not working with
missing 1 required keyword-only argument: 'cursor'
. My simple Schema:class A(Base): __tablename__ = "a" id: Mapped[str] = mapped_column(primary_key=True, index=True) bs: Mapped[List["B"]] = relationship(back_populates="a") class B(Base): __tablename__ = "b" id: Mapped[str] = mapped_column(primary_key=True, index=True) a_id: Mapped[str] = mapped_column(ForeignKey("a.id"), nullable=False) a: Mapped[Cabinet] = relationship(back_populates="bs")
Downgrading from 0.4.2 to 0.3.1 fixes relationships for the time being. I don't know the project enough yet to help you on this.
Same problem, same resolution
Friendly bump, the issue prevents lib update.
Friendly bump, the issue prevents lib update.
Will take a look at this during the weekend.
@erikwrede @mattalbr could you take a look at my comment in https://github.com/strawberry-graphql/strawberry-sqlalchemy/issues/97#issuecomment-1881941667?
I'm leaning towards the first option in there, but I would appreciate your thoughts 😊
python 3.11 sqlalchemy 2.0.23 strawberry-graphql 0.216.0 strawberry-sqlalchemy-mapper 0.4.0
Trying to use this mapper and was playing around with some basics but can't get relationships to work. It doesn't seem like i do something wrong, this is literally the example from readme:
models.py
app.py
main.py
Got this error:
Upvote & Fund