Configuration examples are wrong about kaffe HOT 20 CLOSED

I agree. Maybe they don't even know it and might be worth opening an issue in their repo.

from kaffe.

I tried both these configs and they work perfectly:

config :kaffe,
  consumer: [
    endpoints: [{"kafka1", 19092}, {"kafka2", 29092}, {"kafka3", 39092}],
    topics: ["test"],
    consumer_group: "your-app-consumer-group",
    message_handler: MessageProcessor,
    worker_allocation_strategy: :worker_per_topic_partition,
  ]

config :kaffe,
  consumer: [
    endpoints: [kafka1: 19092, kafka2: 29092, kafka3: 39092],
    topics: ["test"],
    consumer_group: "your-app-consumer-group",
    message_handler: MessageProcessor,
    worker_allocation_strategy: :worker_per_topic_partition,
  ]

Let me know if you want additional logs from the iex shell.

from kaffe.

Also there is slight typo in the README. The MessageProcessor defmodule line is missing do.

defmodule MessageProcessor do
  def handle_messages(messages) do
    for %{key: key, value: value} = message <- messages do
      IO.inspect message
      IO.puts "#{key}: #{value}"
    end
    {:ok, :no_commit}
  end
end

from kaffe.

Hi.
I had no idea about the atom() part in this spec util today:
hostname() = atom() | string().
see http://erlang.org/doc/man/inet.html#type-hostname
When implementing kafka_protocol 2.0, I tried to be Elixir friendly by allowing binary()
but instead missed atom().
Will fix it.

from kaffe.

@zmstone Thanks very much! I still plan to merge the PR above since that's the lingua franca of Brod and it will resolve the issue, I expect, for all Brod versions. But your fix will be good as well.

from kaffe.

@TakteS Thanks for reporting this!

This looks like a compatibility issue with the latest version of Brod. Using Kaffe 1.9.0 and Brod 3.4.0, I don't have the issue. But if I upgrade Brod to latest (3.6.2), I can reproduce your issue.

We'll definitely look into this. In the mean time, you can do what you have above or lock Brod to 3.4.x:

  defp deps do
    [
      {:kaffe, "~> 1.9"},
      {:brod, "~> 3.4.0"},
      ...
    ]
  end

Want to give that a try?

from kaffe.

What about converting host to string on kaffe side?

from kaffe.

That's definitely an option. I'd like to understand the origin of the change first before landing on a solution. It seems that using a string doesn't work with Brod 3.4.0. So it's not an easy change in that sense.

from kaffe.

came across this today. Kaffe wants to use ~> 3.0 compliant 3.7.1".
could Kaffe's mix.exs be set to ~> 3.4.0 until the compatibility is resolved?

from kaffe.

@jdewar I think that makes sense.

from kaffe.

I'm not sure if there was any traction on this but we ran into this issue. The problem is their API changed and you have to now specify Kafka hosts as list of tuples instead of a keyword list.

kafka_hosts = [{"kafka1", 19092}, {"kafka2", 29092}, {"kafka3", 39092}]

You can always convert it like this:

kafka_hosts = [kafka1: 19092, kafka2: 29092, kafka3: 39092]
  |> Enum.map(fn {host, port} -> {Atom.to_string(host), port} end)

It works for us with brod 3.7.9 and the producer in this example: https://github.com/klarna/brod/wiki/Brod-group-subscriber-demo-in-Elixir

from kaffe.

@hackmad Does that work with the older version of Brod as well? Would be awesome to have a solution that worked with old and new versions of Brod.

from kaffe.

Unfortunately it doesn't work with the older version (tested on 3.4.0).

from kaffe.

Seems like a Brod defect to me if it's following semver.

from kaffe.

Added an issue here.

from kaffe.

Hello! I think that strings is more suitable for hosts than atoms. Atom looks ok when the host is :kafka or :localhost, but in case when host is something like kafka.myserver.com string would looks much better (IMO).

Also, why can't you just change the config format for new versions of kaffe? It is not a big incompatibility (especially considering that kaffe is brod's wrapper)

from kaffe.

It would break everyone using atoms. I'd like to find out what broke and why before committing to a fix especially one that breaks backward compatibility.

from kaffe.

I created #91 to fix the compatibility.

@TakteS @hackmad Would you be able to take a look and try it in your configurations?

from kaffe.

@hackmad Thanks for checking it out!

I'll also fix the README.

from kaffe.

kafka4beam/kafka_protocol#56

from kaffe.