florinpatrascu / bolt_sips

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

(Q&A) steps towards releasing Bolt.Sips version 1.3, stable #57

Closed florinpatrascu closed 5 years ago

florinpatrascu commented 5 years ago

@dominique-vassard - we use the -rc postfix mostly to stay close to the db_connection's own status. Now that db_connection is stable, should we too drop the -rc? What do you think?

dominique-vassard commented 5 years ago

There's only one bug with Duration to fix. nanoseconds are not well treated right now:

Regarding json encoding, we could provide some default jason/poison encoder. It could be helpful for those who just want to send back database data via their endpoint without having to develop their own formating module. I agree that, conceptually, it is not part of the driver but it eases its use. Either we can add it, it's as simple as this:

defmodule Bolt.Sips.TypesJasonEncoding do
  alias Bolt.Sips.Types

  defimpl Jason.Encoder, for: Types.DateTimeWithTZOffset do
    def encode(struct, opts) do
      {:ok, dt} = Types.DateTimeWithTZOffset.format_param(struct)
      Jason.Encode.string(dt, opts)
    end
  end

  defimpl Jason.Encoder, for: Types.TimeWithTZOffset do
    def encode(struct, opts) do
      {:ok, dt} = Types.TimeWithTZOffset.format_param(struct)
      Jason.Encode.string(dt, opts)
    end
  end

  defimpl Jason.Encoder, for: Types.Duration do
    def encode(struct, opts) do
      {:ok, dt} = Types.Duration.format_param(struct)
      Jason.Encode.string(dt, opts)
    end
  end

  defimpl Jason.Encoder, for: Types.Point do
    def encode(struct, opts) do
      {:ok, pt} = Types.Point.format_param(struct)
      Jason.Encode.map(pt, opts)
    end
  end
end

But for the release, it can b shipped as soon as the first point is fixed.

florinpatrascu commented 5 years ago

it all makes sense, totally. Tho' for the Bolt.Sips.TypesJasonEncoding, I'd opt for a slightly more generic/cleaner naming convention, i.e.: Bolt.Sips.TypesEncoder, a protocol, for example:

defprotocol Bolt.Sips.TypesEncoder do
  @fallback_to_any true

  def encode(value, opts)
end

something like that. Them we can provide specialized encoders (using a user json library config param), as you very well described above. Or graphql, in the future ;)

Gollor commented 5 years ago

@dominique-vassard I like this approach. It looks pretty well and as I get it still allows the user to override Jason.Encoder implementation and use his own methods.

dominique-vassard commented 5 years ago

it all makes sense, totally. Tho' for the Bolt.Sips.TypesJasonEncoding, I'd opt for a slightly more generic/cleaner naming convention, i.e.: Bolt.Sips.TypesEncoder, a protocol, for example:

defprotocol Bolt.Sips.TypesEncoder do
  @fallback_to_any true

  def encode(value, opts)
end

something like that. Them we can provide specialized encoders (using a user json library config param), as you very well described above. Or graphql, in the future ;)

A generic solution is quite appealing, especially if we consider covering multiple special graph formats (graphML, gexf, cytoscape-schaped data, dot, etc.). For this purpose, a Protocol is the de facto solution.

However, regarding json, it is slightly different if we want a seamless implementation in elixir context. The best example is Phoenix: with the protocol solution, you have to, at some point, call Bolt.Sips.TypesEncoder.encode on your query result. Having some derivations of Poison/Jason protocol allows the user to do, well, nothing :) because call to encoding function will be automatic, which is pretty nice. I've tested it this afternoon and the possibility to treat Neo4j's data as easy as postgres data is great.

Then, we could get the best of both worlds by:

It seems to be more complicated than I thought on first sight, so maybe it's a good idea to release the 1.2 and to work on this feature next, what do you think?

florinpatrascu commented 5 years ago

maybe it's a good idea to release the 1.2 and to work on this feature next, what do you think?

this indeed makes the most sense to me.

dominique-vassard commented 5 years ago

Hi, I've pushed a draft request for some proposals

dominique-vassard commented 5 years ago

We're ok for release 1.3?

florinpatrascu commented 5 years ago

Yes, I believe so. It’s just that I’m away from computer and can’t publish right now. The merge was done from my mobile.

florinpatrascu commented 5 years ago

releasing 1.3.0. Thank you so much!

florinpatrascu commented 5 years ago

1.3.0@Hex.pm, finally :))