Skip to content

Conversation

@efcasado
Copy link
Contributor

@efcasado efcasado commented Apr 2, 2018

This is a small pull-request that aims at setting sane defaults for KafkaEx's SSL settings.

The current implementation requires KafkaEx to explicitly set :use_ssl to either true or false. Otherwise, KafkaEx yields an error at boot time.

iex -S mix
Erlang/OTP 20 [erts-9.1.3] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:10] [hipe] [kernel-poll:false] [dtrace]

=INFO REPORT==== 2-Apr-2018::14:47:51 ===
    application: logger
    exited: stopped
    type: temporary
** (Mix) Could not start application kafka_ex: exited in: KafkaEx.start(:normal, [])
    ** (EXIT) an exception was raised:
        ** (FunctionClauseError) no function clause matching in KafkaEx.Config.ssl_options/2
            (kafka_ex) lib/kafka_ex/config.ex:39: KafkaEx.Config.ssl_options(nil, nil)
            (kafka_ex) lib/kafka_ex.ex:490: KafkaEx.build_worker_options/1
            (kafka_ex) lib/kafka_ex.ex:68: KafkaEx.create_worker/2
            (kafka_ex) lib/kafka_ex.ex:539: KafkaEx.start/2
            (kernel) application_master.erl:273: :application_master.start_it_old/4

The only required change would be L19. However, I took the liberty to set :ssl_options default value to an empty list, which is what the comments suggest.

# ssl_options should be an empty list by default if use_ssl is false

By doing this, we can safely remove the following ssl_option/2's function clause, which becomes dead code.

defp ssl_options(false, nil), do: []

@sourcelevel-bot
Copy link

Hello, @efcasado! 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.

@dantswain dantswain requested a review from bjhaid April 2, 2018 15:57
@joshuawscott joshuawscott merged commit 091c648 into kafkaex:master Apr 2, 2018
@joshuawscott
Copy link
Member

Thanks!

robotarmy pushed a commit to RAM9/kafka_ex that referenced this pull request Apr 7, 2025
Set sane default values for SSL settings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants