Please improve README documentation for 2.0.0

[sriedel]

Background

As a relative newbie to elixir and ecto, I've just tried setting up my first project to test ecto and mongodb. While setting up the project I've had a look at both Ectos "Getting Started" documentation on hex.pm and the Readme in the master branch of this project. I've chosen to use mongodb_ecto 2.0.0-rc.1, since I'm starting fresh anyway. :)

So the following is from the perspective of someone who isn't steeped in background knowledge of the individual libraries and is just trying to get something up and running.

This is with a freshly created project using elixir 1.17.3.

Typo

Small typo in the "Example" Section - the password should probably be mongodb not mongosb

Repo configuration

Then the "Example" section states

# In your application code
defmodule Repo do
  use Ecto.Repo, otp_app: :my_app
end

while "Usage" states

defmodule MyApp.Repo do
  use Ecto.Repo,
    otp_app: :my_app,
    adapter: Mongo.Ecto
end

Turns out, adapter is required, otherwise a compile error is thrown (** (ArgumentError) missing :adapter option on use Ecto.Repo)

Model Configuration

The example section contains a use Ecto.Model while ecto itself seems to use use Ecto.Schema. Not sure what the differences are, I'm assuming Ecto.Model is a past implementation and nowadays Ecto.Schema is the thing to use?

Also, it should be pointed out that the @primary_key {:id, :binary_id, autogenerate: true} is important, otherwise one will get a ** (RuntimeError) MongoDB adapter does not support :id type as primary key when trying to persist a document.

mix.exs: applications or extra_applications?

The "Usage" section states that ecto and mongodb_ecto should be added to the applications configuration, next to logger. However mix created the configuration with logger in extra_applications. Is this where ecto and mongodb_ecto should be placed, or is applications the correct place? Does one need to go in one, and the other in the other? Not sure. I've put both in extra_applications, and it seems to work (in iex at least).

mix ecto.create

Using the config example in the "Example" section, mix ecto.create said

warning: could not find Ecto repos in any of the apps: [:ecto_mongodb_test]
You can avoid this warning by passing the -r flag or by setting the
repositories managed by those applications in your config/config.exs:

    config :ecto_mongodb_test, ecto_repos: [...]

I had created MyRepo with mix ecto.gen.repo with the -r option, so that was confusing at first.
Changing the config section to look as follows:

config :ecto_mongodb_test, MyRepo,
  database: "ecto_mongodb_test_my_repo",
  hostname: "localhost",
  ecto_repos: [MyRepo]

did not fix the error, however. I actually needed to remove the MyRepo reference for mix ecto.create to be happy:

config :ecto_mongodb_test,
  database: "ecto_mongodb_test_my_repo",
  hostname: "localhost",
  ecto_repos: [MyRepo]

However, trying to start iex -S mix now floods my terminal with the message:

09:51:38.102 [error] :gen_statem #PID<0.359.0> terminating
** (RuntimeError) connect raised KeyError exception: key :database not found. The exception details are hidden, as they may contain sensitive data such as database credentials. You may set :show_sensitive_data_on_connection_error to true when starting your connection if you wish to see all of the details
    (elixir 1.17.3) lib/keyword.ex:602: Keyword.fetch!/2
    (mongodb_driver 1.4.1) lib/mongo/mongo_db_connection.ex:35: Mongo.MongoDBConnection.connect/1
    (db_connection 2.7.0) lib/db_connection/connection.ex:74: DBConnection.Connection.handle_event/4
    (stdlib 6.1.2) gen_statem.erl:3737: :gen_statem.loop_state_callback/11
    (stdlib 6.1.2) proc_lib.erl:329: :proc_lib.init_p_do_apply/3
Queue: [internal: {:connect, :init}]
Postponed: []
State: Mongo.MongoDBConnection
Callback mode: :handle_event_function, state_enter: false

which was confusing, as the :database key was in the config section. Adding it to the use Ecto.Repo declaration in MyRepo didn't fix it. I did need to re-add the MyRepo to the config section to fix this:

config :ecto_mongodb_test, MyRepo,
  database: "ecto_mongodb_test_my_repo",
  hostname: "localhost",
  ecto_repos: [MyRepo]

Now iex -S mix starts up and I can interact with mongodb. So it seems I can either issue database queries or interact with mix ecto commands, but not both without changing the config? This doesn't feel right :).

mix mongo.migrate

Some mongo mix tasks are added, but are not mentioned in the Readme, while the ecto documentation uses examples of mix ecto.gen.migration, which seems to be completely missing.

I realize that these mix tasks are probably database specific, a pointer to the mongodb_drivers documentation of migrations would be helpful, just to know what format the migrations need to be in.

Dependency update to mongodb_driver 1.5.0?

mongodb_driver 1.5.0 was released in September, while the release candidate is stuck on ~> 1.4.0. This seems to have been fixed - could an rc.3 version be released that includes this fix?

decimal dependencies

I made the mistake of first adding :ecto as a dependency (without mongodb_ecto ), running mix deps.get, then adding mongodb_ecto and then encountering a versioning issue with decimal. The initial mix deps.get locked the version to 2.3.0 which is incompatible with mongodb_drivers requirement of ~> 2.1.1.

It took me a while to realize that the solution was to:

mix deps.unlock --all
mix deps.update --all

which fixed the issue. Typical newbie trouble, and not immediateely related to this project. But I thought I'd mention it, since some other newbie may hit that snag and giving the solution here will help them.

[Edit: fixed decimal dependency name]

[scottmessinger]

@sriedel I can't thank you enough for all this! These are excellent points. We'll try to get these fixed. /cc @mweidner037

[scottmessinger]

@mweidner037 I think this might be necessary changes for a 2.0 release. What do you think?

[mweidner037]

@mweidner037 I think this might be necessary changes for a 2.0 release. What do you think?

It would be easier to add to release 2.1, to avoid cross-wiring our git history. Or, we cancel #199 and use the current master as 2.0 - the disadvantage being that then 2.0 will not work with Ecto versions < 3.12 (#198).

[mweidner037]

@sriedel Regarding ecto commands (e.g. mix ecto.create): I believe you need two separate configuration blocks, one for the Repo:

config :ecto_mongodb_test, MyRepo,
  database: "ecto_mongodb_test_my_repo",
  hostname: "localhost",

and one for the application as whole:

config :ecto_mongodb_test,
  ecto_repos: [MyRepo]

I am not too familiar with ecto commands, but that setup is what works in our codebase.

[scottmessinger]

@mweidner037 Could we fix up the readme in master for 2.0, revert #198, cut 2.0, and then merge in #198 and #199? I'd like people to have a proper 2.0 that doesn't require 3.12.

[mweidner037]

@sriedel

Some mongo mix tasks are added, but are not mentioned in the Readme, while the ecto documentation uses examples of mix ecto.gen.migration, which seems to be completely missing.

I realize that these mix tasks are probably database specific, a pointer to the mongodb_drivers documentation of migrations would be helpful, just to know what format the migrations need to be in.

I believe this is covered in the full module docs (https://hexdocs.pm/mongodb_ecto/Mongo.Ecto.html#module-indexes-and-migrations), though I do not know how up-to-date that is. I will add a link to the bottom of the readme so that it is clear there is more info elsewhere.

[mweidner037]

@mweidner037 Could we fix up the readme in master for 2.0, revert #198, cut 2.0, and then merge in #198 and #199? I'd like people to have a proper 2.0 that doesn't require 3.12.

Yes, that should be fine.