inspirehep / inspire-next

The INSPIRE repo.
https://inspirehep.net
GNU General Public License v3.0
59 stars 69 forks source link

migrator: unregister_signals doesn't seem to unregister signals #2770

Closed chris-asl closed 6 years ago

chris-asl commented 6 years ago

In migrator/tasks module, the unregister_signal doesn't seem to unregister the signals, because the CollectionUpdater receiver is registered.

Current Behavior

Currently, in after running the unregister_signals(), we can see the registered signals:

(pdb) signals.before_record_insert.receivers
{4412252240: <weakref at 0x108e33460; to 'function' at 0x106fda050 (assign_phonetic_block)>,  
 4444116224: <weakref at 0x108e33530; to 'function' at 0x108e3d500 (assign_uuid)>,  
 4458247760: <invenio_collections.receivers.CollectionUpdater object at 0x109bb7650>}
jacquerie commented 6 years ago

This is because invenio-collections is registered twice to the application. Once here: https://github.com/inveniosoftware/invenio-collections/blob/f3adca45c6d00a4dbf1f48fd501e8a68fe347f2f/setup.py#L129-L131 and another time here: https://github.com/inspirehep/inspire-next/blob/4fe66e7e36b2d2068851814cf13206ef405d91f4/setup.py#L200. In fact, here's what we see:

>>> from invenio_records.signals import before_record_insert
>>> before_record_insert.receivers

{104288936: <weakref at 0x6364b48; to 'function' at 0x63752a8 (assign_phonetic_block)>,
 104289056: <weakref at 0x6364c18; to 'function' at 0x6375320 (assign_uuid)>,
 121678928: <invenio_collections.receivers.CollectionUpdater at 0x740ac50>,
 132516176: <invenio_collections.receivers.CollectionUpdater at 0x7e60950>}

moreover, we can't just call current_collections.unregister_signals() twice, because it uses a reference to the function it should remove in current_collections.update_function, and a reference to the first copy has been lost forever because we called again current_collections.register_signals(). See:

>>> from invenio_collections import current_collections
>>> current_collections.update_function

<invenio_collections.receivers.CollectionUpdater at 0x740ac50>

>>> current_collections.unregister_signals()
>>> before_record_insert.receivers

{104288936: <weakref at 0x6364b48; to 'function' at 0x63752a8 (assign_phonetic_block)>,
 104289056: <weakref at 0x6364c18; to 'function' at 0x6375320 (assign_uuid)>,
 132516176: <invenio_collections.receivers.CollectionUpdater at 0x7e60950>}

>>> current_collections.unregister_signals()
>>> before_record_insert.receivers

{104288936: <weakref at 0x6364b48; to 'function' at 0x63752a8 (assign_phonetic_block)>,
 104289056: <weakref at 0x6364c18; to 'function' at 0x6375320 (assign_uuid)>,
 132516176: <invenio_collections.receivers.CollectionUpdater at 0x7e60950>}

This, by the way, explains why invenio_collections was SUCH a performance problem: it's expensive to run this receiver one time, and we were running it two times...

CC: @chris-asl @david-caro @jmartinm

jacquerie commented 6 years ago

This fix suggested by @david-caro should do the trick: https://github.com/inveniosoftware/invenio-collections/pull/84.

jacquerie commented 6 years ago

This fix suggested by @david-caro should do the trick: inveniosoftware/invenio-collections#84.

It doesn't. Here's the reason: https://github.com/inveniosoftware/invenio-collections/pull/84#issuecomment-330983615