nielsgl / sequelize-paper-trail

Sequelize plugin for tracking revision history of model instances.
MIT License
94 stars 69 forks source link

Support for upserts? #61

Open Lemmmy opened 5 years ago

Lemmmy commented 5 years ago

Currently, it does not look like sequelize-paper-trail supports revisions for upserts (and possibly bulkCreate too?). Would it be possible/feasible to support this, or should I manually do findOne + insert/update? I'm using MySQL.

ksfreitas commented 5 years ago

Paper trail uses sequelize hooks. This is possible setting { individualHooks: true } at bulk operation.

http://docs.sequelizejs.com/manual/tutorial/hooks.html

Lemmmy commented 5 years ago

From what I can see, sequelize-paper-trail does not use the before/afterUpsert hooks:

https://github.com/nielsgl/sequelize-paper-trail/blob/e9b48cc1178689b6f5b0f73a3e71dee14b09fcb4/lib/index.js#L199-L204

Lemmmy commented 5 years ago

One problem with using { individualHooks: true } is when creating a lot of records in bulk (like you're supposed to). sequelize-paper-trail will try to insert tens of thousands of create revisions one-by-one, which can lead to a large number of inserts failing/timing out due to pool inavailability, etc.

Executing (default): INSERT INTO `Revisions` (...)
Executing (default): INSERT INTO `Revisions` (...)
Executing (default): INSERT INTO `Revisions` (...)
Executing (default): INSERT INTO `Revisions` (...)
Executing (default): INSERT INTO `Revisions` (...)
Revision save error
{ TimeoutError: ResourceRequest timed out
    at ResourceRequest._fireTimeout (node_modules/generic-pool/lib/ResourceRequest.js:62:17)
    at Timeout.bound (node_modules/generic-pool/lib/ResourceRequest.js:8:15)
    at ontimeout (timers.js:466:11)
    at tryOnTimeout (timers.js:304:5)
    at Timer.listOnTimeout (timers.js:267:5) name: 'TimeoutError' }
Revision save error
{ TimeoutError: ResourceRequest timed out
    at ResourceRequest._fireTimeout (node_modules/generic-pool/lib/ResourceRequest.js:62:17)
    at Timeout.bound (node_modules/generic-pool/lib/ResourceRequest.js:8:15)
    at ontimeout (timers.js:466:11)
    at tryOnTimeout (timers.js:304:5)
    at Timer.listOnTimeout (timers.js:267:5) name: 'TimeoutError' }
Revision save error
{ TimeoutError: ResourceRequest timed out
    at ResourceRequest._fireTimeout (node_modules/generic-pool/lib/ResourceRequest.js:62:17)
    at Timeout.bound (node_modules/generic-pool/lib/ResourceRequest.js:8:15)
    at ontimeout (timers.js:466:11)
    at tryOnTimeout (timers.js:304:5)
    at Timer.listOnTimeout (timers.js:267:5) name: 'TimeoutError' }
Revision save error
{ TimeoutError: ResourceRequest timed out
    at ResourceRequest._fireTimeout (node_modules/generic-pool/lib/ResourceRequest.js:62:17)
    at Timeout.bound (node_modules/generic-pool/lib/ResourceRequest.js:8:15)
    at ontimeout (timers.js:466:11)
    at tryOnTimeout (timers.js:304:5)
    at Timer.listOnTimeout (timers.js:267:5) name: 'TimeoutError' }
nielsgl commented 5 years ago

@Lemmmy @ksfreitas any ideas on how to implement it / whether it's worth to implement it?

mihajul commented 2 years ago

Support for upsert opperation PR: https://github.com/nielsgl/sequelize-paper-trail/pull/123