Skip to content
This repository has been archived by the owner on Jun 30, 2021. It is now read-only.

Surround Transfer retrieval after insert with transaction #219

Merged
merged 2 commits into from
Jun 5, 2018

Conversation

T-Dnzt
Copy link

@T-Dnzt T-Dnzt commented May 29, 2018

Issue/Task Number: T347

Overview

Since we have multiple nodes of the DB, there seem to be some randomly failing method in the Transfer insert flow. Basically, the retrieval after the insert fails because the record was "potentially" inserted in another node. This PR surrounds the insert/get flow with a transaction as an attempt to fix that.

Changes

  • Add Repo.transaction() around in insert

@T-Dnzt T-Dnzt requested review from sirn and unnawut May 29, 2018 03:53
@T-Dnzt T-Dnzt added the kind/bug ⚠️ Something isn't working label May 29, 2018
@T-Dnzt T-Dnzt force-pushed the T347-insert-load-fail branch 2 times, most recently from 506cedb to a935730 Compare May 29, 2018 03:58
|> changeset(attrs)
|> do_insert(opts)
|> case do
{_, res} -> res
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would the new return types cause problem? E.g. before, it would return {:ok, result} or changeset. Now it's {:ok, result} or {:error, changeset}?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

insert actually returns {:error, changeset}, here it's just shortcuted to changeset. Insert doc

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sorry changeset made me immediately thought it's returning %Ecto.Changeset{}. Ignore me.

|> changeset(attrs)
|> do_insert(opts)
|> case do
{_, res} -> res
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sorry changeset made me immediately thought it's returning %Ecto.Changeset{}. Ignore me.

@T-Dnzt T-Dnzt merged commit e4df115 into develop Jun 5, 2018
@T-Dnzt T-Dnzt deleted the T347-insert-load-fail branch June 5, 2018 05:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug ⚠️ Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants