Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always save sync_process #501

Open
kkolotyuk opened this issue Oct 31, 2016 · 2 comments
Open

Always save sync_process #501

kkolotyuk opened this issue Oct 31, 2016 · 2 comments

Comments

@kkolotyuk
Copy link
Contributor

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
Copy link
Contributor

keang commented Nov 2, 2016

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

@kkolotyuk
Copy link
Contributor Author

kkolotyuk commented Nov 2, 2016

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

  • download all properties and sync them one by one

another

  • download properties group by group (and we should skip_purge if one group fails)

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants