-
Notifications
You must be signed in to change notification settings - Fork 428
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Service and module options only in maps #3625
Conversation
Also: pass a map to mongoose_backend
1fa34d3
to
c554a2e
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Codecov Report
@@ Coverage Diff @@
## master #3625 +/- ##
==========================================
- Coverage 81.00% 76.26% -4.75%
==========================================
Files 427 427
Lines 32126 32098 -28
==========================================
- Hits 26025 24479 -1546
- Misses 6101 7619 +1518
Continue to review full report at Codecov.
|
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
47758fe
to
3bdde49
Compare
This comment was marked as outdated.
This comment was marked as outdated.
mod_websockets is actually not a gen_mod. It could be renamed.
This module is used for modules that are not gen_mod's, thus maps are more appropriate.
Make the option-less modules use an empty map
This comment was marked as outdated.
This comment was marked as outdated.
72abadc
to
ee1a9a0
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
11ab32b
to
b220bc2
Compare
This comment was marked as outdated.
This comment was marked as outdated.
- Add lookup_module_opts to gen_mod It is less error-prone than plugging in 'undefined' as default - Use mongoose_config directly in gen_mod getters Also: - improve test coverage - use the already existing check for is_exported
Remove the default and use maps - done for simple cases, as some pubsub logic is obtaining options for non-existing hosts. This would require more substantial rework.
mod_pubsub is the last module to use the dynamic compilation, and it is doing so in a direct way.
Also: add missing iqdisc option
Previously defaults were used even if MUC(Light) was not enabled.
b220bc2
to
0e64090
Compare
This comment was marked as outdated.
This comment was marked as outdated.
small_tests_24 / small_tests / 840ee05 small_tests_23 / small_tests / 840ee05 dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 840ee05 dynamic_domains_mysql_redis_24 / mysql_redis / 840ee05 sm_SUITE:parallel_manual_ack_freq_1:resume_session_state_send_message_without_ack{error,
{{assertion_failed,assert,is_presence,
{xmlel,<<"message">>,
[{<<"from">>,
<<"bOb_resume_session_state_send_message_without_ack_1926@domain.example.com/escalus-default-resource">>},
{<<"to">>,
<<"alicE_resume_session_state_send_message_without_ack_1935@domain.example.com">>},
{<<"xml:lang">>,<<"en">>},
{<<"type">>,<<"chat">>}],
[{xmlel,<<"body">>,[],[{xmlcdata,<<"msg-3">>}]},
{xmlel,<<"delay">>,
[{<<"xmlns">>,<<"urn:xmpp:delay">>},
{<<"stamp">>,<<"2022-04-07T12:20:40.754781Z">>},
{<<"from">>,<<"domain.example.com">>}],
[{xmlcdata,<<"Offline Storage">>}]}]},
"<message from='bOb_resume_session_state_send_message_without_ack_1926@domain.example.com/escalus-default-resource' to='alicE_resume_session_state_send_message_without_ack_1935@domain.example.com' xml:lang='en' type='chat'><body>msg-3</body><delay xmlns='urn:xmpp:delay' stamp='2022-04-07T12:20:40.754781Z' from='domain.example.com'>Offline Storage</delay></message>"},
[{escalus_new_assert,assert_true,2,
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_new_assert.erl"},
{line,84}]},
{sm_helper,initial_presence_step,2,
[{file,"/home/circleci/project/big_tests/tests/sm_helper.erl"},
{line,133}]},
{escalus_connection,connection_step,2,
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escal... dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 840ee05 dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 840ee05 ldap_mnesia_23 / ldap_mnesia / 840ee05 ldap_mnesia_24 / ldap_mnesia / 840ee05 internal_mnesia_24 / internal_mnesia / 840ee05 pgsql_mnesia_24 / pgsql_mnesia / 840ee05 elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 840ee05 pgsql_mnesia_23 / pgsql_mnesia / 840ee05 mysql_redis_24 / mysql_redis / 840ee05 mssql_mnesia_24 / odbc_mssql_mnesia / 840ee05 riak_mnesia_24 / riak_mnesia / 840ee05 dynamic_domains_mysql_redis_24 / mysql_redis / 840ee05 |
small_tests_24 / small_tests / d539679 small_tests_23 / small_tests / d539679 dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / d539679 dynamic_domains_mysql_redis_24 / mysql_redis / d539679 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / d539679 ldap_mnesia_23 / ldap_mnesia / d539679 ldap_mnesia_24 / ldap_mnesia / d539679 internal_mnesia_24 / internal_mnesia / d539679 pgsql_mnesia_24 / pgsql_mnesia / d539679 dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / d539679 pgsql_mnesia_23 / pgsql_mnesia / d539679 mysql_redis_24 / mysql_redis / d539679 elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / d539679 mssql_mnesia_24 / odbc_mssql_mnesia / d539679 riak_mnesia_24 / riak_mnesia / d539679 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. I am not experienced in this area though, but I tried to keep up with the changes :) I have one small comment.
The main change is to accept only lists for module and service options. There are multiple related small corrections and fixes as well:
mongoose_config
more directly ingen_mod
, addlookup_module_opts
to encourage using it instead of addingundefined
as the default valuegen_mod:get_*
for anything different than module options - useproplists
ormaps
depending on the data structuregen_mod:start_backend_module/3
and usebackend_module
directly in the last unconverted module, which ismod_pubsub
. There is no benefit in usinggen_mod
here.