instedd / surveda

InSTEDD Surveda
https://instedd.org/technologies/surveda-mobile-surveys/
GNU General Public License v3.0
16 stars 6 forks source link

#2330: lock respondents in ChannelBroker #2334

Closed anaPerezGhiglia closed 1 month ago

anaPerezGhiglia commented 2 months ago

Main changes

Bonus track

closes #2330

ysbaddaden commented 2 months ago

@anaPerezGhiglia Is this meant to ensure that another actor won't modify the respondent while we're trying to contact them? Otherwise only the expired callback is supposed to update the respondent (ask/setup shouldn't).

anaPerezGhiglia commented 2 months ago

Is this meant to ensure that another actor won't modify the respondent while we're trying to contact them? Otherwise only the expired callback is supposed to update the respondent (ask/setup shouldn't).

@ysbaddaden yes, it's true that nuntium_channel and verboice_channel do not end up modifying the respondent, but actually the channel_broker doesn't know that... that's why I decided to leave the Respondent.with_lock for both scenarios (ivr and sms)

ysbaddaden commented 2 months ago

I'd consider it surprising for the channels to modify the respondent, I'd expect them to try to push the contact and failures to bubble back to the channel broker. But just protecting against a parallel modification is a good enough reason to lock it!

Just in case: expiring the contact doesn't also try to lock the respondent (or it's reentrant)?