florinpatrascu / bolt_sips

Neo4j driver for Elixir
Apache License 2.0
256 stars 49 forks source link

A valid code example for Elixir 1.5 #36

Closed dnesteryuk closed 6 years ago

dnesteryuk commented 6 years ago

The 0.5.x version of Bolt.Sips implements the module-based child spec (https://hexdocs.pm/elixir/Supervisor.html), thus, it follows a new way of running processes in a supervisor tree. This change makes sure the README.md is also up to date. :smile:

sourcelevel-bot[bot] commented 6 years ago

Hello, @dnesteryuk! This is your first Pull Request that will be reviewed by Ebert, an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its review page. Please check our documentation for more information.

dnesteryuk commented 6 years ago

Oh, actually, the updated code example works fine for 0.4.x, because that version also implements the child spec :wink:

florinpatrascu commented 6 years ago

Forgot to mention, I tried this on Heroku (free account) with Elixir 1.5, Erl 20.1 and with the bolt_sips from master, and the demo phoenix (1.2) app throws an awful exception at startup and crashes in flames. Excerpt:

{"Kernel pid terminated",application_controller,"{application_start_failure,movies_elixir_phoenix,{{shutdown,{failed_to_start_child,'Elixir.Bolt.Sips',{#{'__exception__' => true,'__struct__' => 'Elixir.ArgumentError',message => <<\"supervisors expect the child to be a module, a {module, arg} tuple or a map with the child specification, got: {DBConnection.Poolboy, {:poolboy, :start_link, [[name: {:local, :bolt_sips_pool}, strategy: :fifo, size: 5, max_overflow: 1, worker_module: DBConnection.Poolboy.Worker], {Bolt.Sips.Protocol,...

I'd need to debug this further, this weekend.

Similar behavior with 4.11 - using the same Elixir/Erlang stack, as above.

Locally works perfect :(

emilsoman commented 6 years ago

@dnesteryuk Elixir >= 1.4 also automatically starts the dependent applications, so there's no need to explicitly add :bolt_sips to the applications list. How about mentioning "If using Elixir version < 1.4, ...." in the relevant part of the README while you're at it?