basho / riak_core

Distributed systems infrastructure used by Riak.
Apache License 2.0
1.23k stars 391 forks source link

OTP 22 - Dialyzer Warning (exometer) #946

Open martinsumner opened 4 years ago

martinsumner commented 4 years ago

Failure to build > OTP 20

_build/default/lib/riak_core/src/riak_core_ssl_util.erl:131: ssl:ssl_accept/2: deprecated; use ssl:handshake/2 instead

As ssl:handshake was not introduced until OTP21, should we use nowarn_deprecated_function for now?

Also see - https://github.com/basho/riak_core/blob/develop-3.0/src/riak_core_tcp_mon.erl#L461

The ssl:handshake/1 function does not have ok as a valid return - https://github.com/erlang/otp/edit/maint/lib/ssl/doc/src/ssl.xml#L1352.

v-kat commented 4 years ago

This might have been addressed by my PR. Let me know, if you want anything else. It might be nice to have all the supported versions built on travis to have a more automated CI process.

damfly commented 4 years ago

@IRog after using your PR, I confirm it compiles and everything is ok so far while running it

martinsumner commented 4 years ago

Looks good. There is just the one dialyzer issue with exometer, and then we can close the issue - there's someone looking at this now.

Many thanks for the PR @IRog

kshitizsuman-gs commented 4 years ago

@IRog I see you haven't merged your PR changes to the develop branch yet. When are you planning to make a release for riak_core supporting otp 21 and 22 ??

martinsumner commented 4 years ago

There is no active develop branch for riak_core - develop contained work on Riak features which were planned under Basho (but not released), work on which has discontinued. The next release of Riak KV will be based on the develop-3.0 branch which will contain @IRog's changes - and there will be a tagged release of riak_core as part of this.

For a period there will also be tagged releases of riak_core based on the develop-2.9 branches to support the Riak KV 2.9.x releases. All these changes will eventually be merged into develop-3.0.

martinsumner commented 4 years ago

After discussion with @ThomasArts the outstanding dialyzer issue in OTP 22 is going to be resolved through a change to exometer not riak_core. As it is not an issue with riak_core itself, I will close this issue.

martinsumner commented 4 years ago

Leaving the issue open - to be reference in temporary dialyzer override

l2dy commented 3 years ago

Is this still an issue? Riak KV is said to be tested against OTP 22 now. https://github.com/basho/riak/blob/riak-3.0.6/RELEASE-NOTES.md

martinsumner commented 3 years ago

The issue has been left open as although everything works on OTP 22, there's still a dialyzer issue related to exometer where we're waiting for a change in exometer until we are fully clean in OTP 20+.

l2dy commented 3 years ago

The issue has been left open as although everything works on OTP 22, there's still a dialyzer issue related to exometer where we're waiting for a change in exometer until we are fully clean in OTP 20+.

Is it https://github.com/Feuerlabs/exometer_core/pull/122? If we are waiting for a new release from exometer, beware that they have not tagged a new release for two years.

Maybe we should update the title of this issue now that OTP 22 is in fact supported?

martinsumner commented 3 years ago

For note - this is the specific warning (https://github.com/basho/riak_core/blob/develop-3.0/src/riak_core_stat.erl#L23-L26).

I think there was some debate about whether this truly was an exometer issue. We've not pushed hard to fix it, as there has been work underway for a while to potentially replace stats.