post_process method outside batch consumption DB transaction #201
Conversation
harsha-flipp
changed the title
|
|
||
| def post_process(records) | ||
| # ActiveRecord objects that have been used to save to the database. | ||
| # They contain primary keys only if associations are involved. |
There was a problem hiding this comment.
The wording is a bit awkward... maybe
This method gets passed the ActiveRecord objects that were saved to the database.
They contain primary keys only if associations were also saved via bulk_import_id.
There was a problem hiding this comment.
have been used to I am sorry I should not flex my grammar. I will use your words.
I wanted to give the intent that these are not the active records returned after save. These are the records we created and passing to save the record.
There was a problem hiding this comment.
But... aren't they the same thing?
There was a problem hiding this comment.
I misunderstood that rails adds IDs back to the objects for every save.
I tested in rails console and attribute ID is saved to the object only whenobject.save! is called. For other scenarios, object.save doesnot really change the object attribute ID.
| def uncompacted_update(messages) | ||
| BatchSlicer. | ||
| slice(messages). | ||
| each(&method(:update_database)) | ||
| each(&method(:update_database)).flatten |
There was a problem hiding this comment.
Probably should use map instead of each here since we're using the return value.
| expect(batch_consumer).to receive(:post_process).once.with(records) | ||
| expect(batch_consumer).to receive(:update_database) { records } | ||
| batch_consumer.send(:consume_batch, payload, metadata) | ||
| end |
There was a problem hiding this comment.
I think we're probably missing some more tests - e.g. with and without max_db_batch_size, and with and without deletions. Probably can just add another expect onto existing tests showing that they return the expected values.
|
Lionel tackled with these changes in his PR. I am closing this. |
Pull Request Template
Description
After batch consumption transaction is complete(BULK INSERTs), previously nothing is returned. Now we are passing the active record objects created back to consume_batch method so these records objects can be used by
post_processhook method. Consumers can override this to do an action on the Active Record Models.Note that these active record models are used by ActiveRecord-Import gem to bulk insert data. So record IDs are populated only if there is association involved.
Usecase: After saving records to Database, I queue a sidekiq job with record IDs.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
I ran the unit tests locally and this code change adds an empty
post_processmethod in consumer. So it should not hurt any one upgrading to this version. For a meaningful use, they will have to overridepost_processin their consumers.Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist: