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

Introduce Ecto.Adapters.SQL.Stage #2028

Closed
wants to merge 5 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@fishcakez
Member

fishcakez commented Apr 12, 2017

Adds support for a GenStage process to encapsulate a SQL transaction for use with Flow.

This is WIP.

  • Add examples
  • Improve test coverage
  • Add wrapper for Postgrex.Stage.copy/4
  • Merge related PRs

Related PRs:
elixir-ecto/db_connection#82
elixir-ecto/postgrex#312

GenServer.on_start when acc: var
def producer(repo, start, handle_demand, stop, opts \\ []) do
fun = &DBConnection.Stage.producer/5
Ecto.Adapters.SQL.stage(fun, repo, start, handle_demand, stop, opts)

This comment has been minimized.

@ebert

ebert bot Apr 12, 2017

There should be no trailing white-space at the end of a line.

{:suspend, {0, [v | acc]}}
end
defp stream_reduce(v, {n, acc}) do
{:cont, {n-1, [v | acc]}}

This comment has been minimized.

@ebert

ebert bot Apr 12, 2017

There are spaces around operators most of the time, but not here.

assert [p1, p2, p3] = stage |> to_list() |> sort_by_id()
assert [%Comment{id: ^cid1}, %Comment{id: ^cid2}] = p1.comments |> sort_by_id
assert [%Comment{id: ^cid3}, %Comment{id: ^cid4}] = p2.comments |> sort_by_id

This comment has been minimized.

@ebert

ebert bot Apr 12, 2017

Use a function call when a pipeline is only one function long

assert [p1, p2, p3] = stage |> to_list() |> sort_by_id()
assert [%Comment{id: ^cid1}, %Comment{id: ^cid2}] = p1.comments |> sort_by_id

This comment has been minimized.

@ebert

ebert bot Apr 12, 2017

Use a function call when a pipeline is only one function long

@josevalim josevalim force-pushed the master branch 8 times, most recently from 8527ce2 to 7aa5c62 Apr 23, 2017

The first argument is the repo, the second argument is the start function,
the third argument is the handle demand function, the fourth argument is the
stop function and the optional fiftth argument are the options.

This comment has been minimized.

@zorbash

zorbash May 7, 2017

typo: fiftth

@fishcakez fishcakez force-pushed the jf-sql-stage branch from d5ddc6a to 56df49d May 29, 2017

@fishcakez

This comment has been minimized.

Member

fishcakez commented May 29, 2017

Changed this up quite a bit, so we can now have what I assume will be far the two most common cases in easy reach, the first acting like all/1, and the second like insert_all/2:

MyRepo.start_producer(Post)
MyRepo.start_consumer(Post)

Also lower level versions in Ecto.Adapters.SQL. However neither Postgrex.CopyConsumer nor checkout_begin etc

@ebert

This comment has been minimized.

ebert bot commented May 29, 2017

Ebert has finished reviewing this Pull Request and has found:

  • 6 possible new issues (including those that may have been commented here).
  • 427 fixed issues! 🎉

But beware that this branch is 46 commits behind the elixir-ecto:master branch, and a review of an up to date branch would produce more accurate results.

You can see more details about this review at https://ebertapp.io/github/elixir-ecto/ecto/pulls/2028.

@josevalim

This comment has been minimized.

Member

josevalim commented Aug 22, 2018

I will go ahead and close this since we have no plans to pick this up.

@josevalim josevalim closed this Aug 22, 2018

@josevalim josevalim deleted the jf-sql-stage branch Oct 22, 2018

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