roomorama / concierge

Roomorama supplier integrations - webhook providers and property synchronisation
https://concierge-web.roomorama.com
3 stars 0 forks source link

Always save sync_process #501

Open kkolotyuk opened 7 years ago

kkolotyuk commented 7 years ago

Currently I see that some calls of metadata workers can even not create sync process in DB. For example SAW sync:

    def perform
      result = synchronisation.new_context { importer.fetch_countries }

      if result.success?
        countries = result.value
      else
        message = "Failed to perform the `#fetch_countries` operation"
        announce_error(message, result)
        return
      end
      ...

My position is that every run of worker should leave something after it (for example successful/unsuccessful sync_process record in DB). So I wanted to make it mandatory to call sync.finish! for all wokers, but we also shouldn't forget to call skip_purge in some error cases, otherwise we can purge all the properties((((

Here how I would fix the SAW's worker:

    def perform
      result = synchronisation.new_context { importer.fetch_countries }

      if result.success?
        countries = result.value
      else
        message = "Failed to perform the `#fetch_countries` operation"
        announce_error(message, result)
        synchronisation.skip_purge!
        synchronisation.finish!
        return
      end
      ...

Please let me know what do you think about this thought.

keang commented 7 years ago

Good point. This issue could really benefit from a shared specs, as discussed earlier right?

kkolotyuk commented 7 years ago

It's still depends on each supplier implementation, some of them

another

I don't have idea how to create shared spec for all the suppliers. But I have a suggestion how we can prevent purging all the properties: before purge we should check if start method was called at least once, if no - don't run the purge.