Skip to content
This repository was archived by the owner on Sep 2, 2026. It is now read-only.

Updated db_connection to v2 - #32

Open
andrewstuarttaxfix wants to merge 5 commits into
masterfrom
updated_db_connection_to_2
Open

andrewstuarttaxfix wants to merge 5 commits into
masterfrom
updated_db_connection_to_2

Conversation

@andrewstuarttaxfix

Copy link
Copy Markdown

The codebase currently uses db_connection v1, which in itself uses Ecto 2. As a result of this, applications that rely on this library cannot upgrade to Ecto 3. This PR updates the codebase to use db_connection v2. The API to external clients stays the same as previously expected.

@liveforeverx liveforeverx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me! Just some Nits comments/questions.

Comment thread lib/grakn.ex Outdated
@spec query(conn(), Grakn.Query.t(), Keyword.t()) :: any()
def query(conn, query, opts \\ []) do
DBConnection.execute(get_conn(conn), query, [], with_transaction_config(opts))
|> case do

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Really Nit:

What do you think about this style thing https://github.com/lexmag/elixir-style-guide#needless-pipeline ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is actually a case for with:

with {:ok, _query, result} <- 
      DBConnection.execute(get_conn(conn), query, [], with_transaction_config(opts)) do
  {:ok, result}
end

Comment thread lib/grakn.ex Outdated
@spec query!(conn(), Grakn.Query.t(), Keyword.t()) :: any()
def query!(conn, %Grakn.Query{} = query, opts \\ []) do
DBConnection.execute!(get_conn(conn), query, [], with_transaction_config(opts))
|> case do

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The same Nit question here :-)

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants