Make postgrex respect env var PGDATABASE like libpq.#239
Make postgrex respect env var PGDATABASE like libpq.#239fishcakez merged 5 commits intoelixir-ecto:masterfrom scouten:pgdatabase-env-var
Conversation
|
Shamelessly modeled after #11. |
fishcakez
left a comment
fishcakez
left a comment
There was a problem hiding this comment.
Thanks, just a couple of minor things.
test/login_test.exs
Outdated
|
|
||
| test "env var default db name" do | ||
| System.put_env("PGDATABASE", "postgrex_test") | ||
| opts = [] |
There was a problem hiding this comment.
sync_connect: true please so start_link blocks to connect
test/login_test.exs
Outdated
| opts = [] | ||
| assert {:ok, pid} = P.start_link(opts) | ||
| assert {:ok, %Postgrex.Result{}} = P.query(pid, "SELECT 123", []) | ||
| System.delete_env("PGDATABASE") |
There was a problem hiding this comment.
If the test fails we won't reset, can you copy the dance done with try do..after.. end and PGPORT later in this file?
|
@fishcakez: Sure thing. It may take me a day or two to get to it, but happy to make the changes. |
|
@scouten this is a feature you want so do as you wish 😄. It took a week for this to get reviewed so we were very slow on this end, sorry. |
|
@fishcakez: I've updated per your suggestions. Please re-review. Thanks! |
fishcakez
left a comment
fishcakez
left a comment
There was a problem hiding this comment.
Looks great, just need to delete some code. Sorry if there was a mixup here, we seem to be testing same thing twice.
| {:ok, pid} = P.start_link(opts) | ||
| assert_receive {:EXIT, ^pid, {%Postgrex.Error{postgres: %{code: :invalid_catalog_name}}, [_|_]}} | ||
| end | ||
| after |
There was a problem hiding this comment.
We only need to test this once, either with or without :sync_connect. I think the :sync_connect example is clearer as we would only needs to assert on thestart_linkreturn value. No need to check for the:EXIT`.
|
@fishcakez: Thanks for the quick review. Just pushed another update. |
|
Thank you ❤️ |
3 participants
I'd like to be able to configure my database name in production via environment variable and this would help. (FWIW I'm an Elixir and Ecto newbie. Happy to make changes if needed.)