-
Notifications
You must be signed in to change notification settings - Fork 58
handle cyclic associations #26
base: master
Are you sure you want to change the base?
Conversation
…ct reload when resolving possible
lib/replicate/loader.rb
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passing the local_id here is going to break backward compatibility unfortunately. Existing programs that implement custom load_replicant method will raise an ArgumentError.
|
Thanks for this. I've run into the problem a few times now. I need to do a bit closer review of the diff and approach but I'm definitely interested in landing something that fixes this. /cc @jbarnette |
|
Any update on this? why not just giving a default |
|
Anything new on this? I just cut a new fork where I've merged this into it, but would prefer to track the gem from the source. |
lib/replicate/loader.rb
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this does go forward, this should be a Hash so non Integer ids don't error.
Conflicts: test/active_record_test.rb
…ences objects, check for overridden load_replicant for backward compat
|
Trying to revive this, merged the rtomayko's current master branch and made two changes:
It would be nice if you could test dump & restore in (non-critical ,-) environments of your real world applications. |
cyclic associations lead to an infinite loop on dump, for instance
I suggest two changes which I have implemented as a PoC: