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

Implement alternative DeferredRelation semantics #1018

Closed
c0ldcalamari opened this issue Jul 19, 2016 · 2 comments
Closed

Implement alternative DeferredRelation semantics #1018

c0ldcalamari opened this issue Jul 19, 2016 · 2 comments

Comments

@c0ldcalamari
Copy link

The semantics behind circular foreign key dependencies get kind of unwieldy when you have more than a few models (and they're spread over multiple files). This is because the DeferredRelation object has to be defined, used, then the other model defined in another file, then set_model has to be called on the original, and then you're left with the object reference dangling around that has no purpose. IE the example in the docs:

# Create a reference object to stand in for our as-yet-undefined Tweet model.
DeferredTweet = DeferredRelation()

class User(Model):
    username = CharField()
    # Tweet has not been defined yet so use the deferred reference.
    favorite_tweet = ForeignKeyField(DeferredTweet, null=True)

class Tweet(Model):
    message = TextField()
    user = ForeignKeyField(User, related_name='tweets')

# Now that Tweet is defined, we can initialize the reference.
DeferredTweet.set_model(Tweet)

It would be better if it all happened in the model definition with an optional parameter given to DeferredRelation. Like:

class User(Model):
    username = CharField()
    # Tweet has not been defined yet so use the deferred reference.
    favorite_tweet = ForeignKeyField(DeferredRelation('Tweet'), null=True)

class Tweet(Model):
    message = TextField()
    user = ForeignKeyField(User, related_name='tweets')

This would remove the need for the extra variable in the global namespace and the coordination of it over multiple files. And since the parameter is optional, it can be fully backwards-compatible with the old syntax.

Thank you!

@coleifer
Copy link
Owner

That's a very nice API. Depending on the class name (a string) is something I try to shy away from, as errors can occur despite the code appearing to be perfectly valid, for instance a typo or you happen to change the model name in one place and not the other.

I like the API, though, and agree that it would be worth supporting if done in a backwards-compatible way. Thanks for the patch, looking at it now.

@coleifer
Copy link
Owner

Awesome. Thanks, I've merged your patch, really appreciate you taking the time to fix this one and implement such a clean patch.

coleifer pushed a commit that referenced this issue Jul 20, 2016
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

No branches or pull requests

2 participants