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

Properties should be inherited #83

Closed
katarinasupe opened this issue Jan 24, 2022 · 3 comments · Fixed by #84
Closed

Properties should be inherited #83

katarinasupe opened this issue Jan 24, 2022 · 3 comments · Fixed by #84
Assignees

Comments

@katarinasupe
Copy link
Contributor

If I don't add property name to the class Stream, like this:

class User(Node):
    name: str = Field(index=True, unique=True, db=memgraph)


class Stream(User):
    id: str = Field(index=True, unique=True, db=memgraph)

then I get the following error:

twitch-app_1     |   File "/app/models.py", line 10, in <module>
twitch-app_1     |     class Stream(User):
twitch-app_1     |   File "/usr/local/lib/python3.8/site-packages/gqlalchemy/models.py", line 314, in __new__
twitch-app_1     |     raise GQLAlchemyDatabaseMissingInFieldError(
twitch-app_1     | gqlalchemy.exceptions.GQLAlchemyDatabaseMissingInFieldError

I expected that the name property would be inherited.

This works for now:


class User(Node):
    name: str = Field(index=True, unique=True, db=memgraph)


class Stream(User):
    name: Optional[str] = Field(index=True, unique=True, db=memgraph, label="User")
    id: str = Field(index=True, unique=True, db=memgraph)
@MasterMedo
Copy link
Contributor

Oh, that's right. Because the database reference is removed from the property name once the User is created. It's an easy fix, when a subclass of Node is being created it can propagate the db object upwards. But then a user would have to define the Streamer as:

class Streamer(User, db):
    pass

The other option is what @jbajic and I talked earlier about, and that is creating a global instance of db that is going to be used everywhere. Very few users are going to use multiple databases, and we should cater the simplicity to the majority.

@katarinasupe
Copy link
Contributor Author

Is it possible to add other properties to the Streamer then?
Because, when I try adding the db argument I get the following error:
TypeError: metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases

@MasterMedo
Copy link
Contributor

Oh, the functionality I mentioned is not implemented, it's a proposal.
But after giving it some thought, I think db arguments can be completely ignored when subclasses are created because the superclasses already created all constraints/indexes. I'll push a fix for this by the end of day.

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.

2 participants