Skip to content
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

Support maps in gen_mod_deps #3555

Merged
merged 2 commits into from
Feb 23, 2022
Merged

Support maps in gen_mod_deps #3555

merged 2 commits into from
Feb 23, 2022

Conversation

chrzaszcz
Copy link
Member

@chrzaszcz chrzaszcz commented Feb 22, 2022

Prepare module deps for module opts in maps - now modules using maps can be deps of other modules.

  • Update dependency processing and resolution.
  • Update dependency format - options are mandatory as both [] and #{} are possible now. This does not seem to be worse in any way, so I think it is a permanent change.
  • Update type specs, improve type reuse.
  • Update small tests to check the opts in maps. The transitional phase should be short and I see no reason to keep both formats in tests - it would be complicated.

Minor changes:

  • Use host types.
  • Call module options opts, not args or params.

@chrzaszcz chrzaszcz force-pushed the module-deps-with-maps branch 3 times, most recently from 39b8906 to f1202cf Compare February 22, 2022 10:18
@codecov
Copy link

codecov bot commented Feb 22, 2022

Codecov Report

Merging #3555 (7b99d7b) into master (a89e2ec) will decrease coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3555      +/-   ##
==========================================
- Coverage   79.26%   79.23%   -0.04%     
==========================================
  Files         420      420              
  Lines       32266    32265       -1     
==========================================
- Hits        25577    25564      -13     
- Misses       6689     6701      +12     
Impacted Files Coverage Δ
src/gen_mod.erl 78.57% <ø> (ø)
src/global_distrib/mod_global_distrib.erl 86.73% <ø> (ø)
...bal_distrib/mod_global_distrib_hosts_refresher.erl 71.69% <ø> (-1.89%) ⬇️
src/global_distrib/mod_global_distrib_utils.erl 64.94% <ø> (ø)
src/mam/mod_mam_meta.erl 95.65% <ø> (ø)
src/mongoose_modules.erl 100.00% <ø> (ø)
src/muc_light/mod_muc_light.erl 85.60% <ø> (ø)
src/event_pusher/mod_event_pusher.erl 65.00% <100.00%> (-22.50%) ⬇️
src/gen_mod_deps.erl 97.14% <100.00%> (+1.36%) ⬆️
src/global_distrib/mod_global_distrib_mapping.erl 98.11% <100.00%> (ø)
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a89e2ec...7b99d7b. Read the comment docs.

@mongoose-im

This comment was marked as outdated.

@esl esl deleted a comment from mongoose-im Feb 22, 2022
@esl esl deleted a comment from mongoose-im Feb 22, 2022
@esl esl deleted a comment from mongoose-im Feb 22, 2022
@mongoose-im
Copy link
Collaborator

mongoose-im commented Feb 22, 2022

small_tests_24 / small_tests / 634c0cc
Reports root / small


small_tests_23 / small_tests / 634c0cc
Reports root / small


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 634c0cc
Reports root/ big
OK: 2721 / Failed: 0 / User-skipped: 239 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 634c0cc
Reports root/ big
OK: 2721 / Failed: 0 / User-skipped: 239 / Auto-skipped: 0


dynamic_domains_mysql_redis_24 / mysql_redis / 634c0cc
Reports root/ big
OK: 2704 / Failed: 0 / User-skipped: 256 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 634c0cc
Reports root/ big
OK: 2721 / Failed: 0 / User-skipped: 239 / Auto-skipped: 0


ldap_mnesia_23 / ldap_mnesia / 634c0cc
Reports root/ big
OK: 1510 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / 634c0cc
Reports root/ big
OK: 1510 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0


internal_mnesia_24 / internal_mnesia / 634c0cc
Reports root/ big
OK: 1551 / Failed: 0 / User-skipped: 349 / Auto-skipped: 0


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 634c0cc
Reports root/ big
OK: 1685 / Failed: 0 / User-skipped: 369 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / 634c0cc
Reports root/ big
OK: 3095 / Failed: 0 / User-skipped: 248 / Auto-skipped: 0


pgsql_mnesia_23 / pgsql_mnesia / 634c0cc
Reports root/ big
OK: 3095 / Failed: 0 / User-skipped: 248 / Auto-skipped: 0


mysql_redis_24 / mysql_redis / 634c0cc
Reports root/ big
OK: 3090 / Failed: 0 / User-skipped: 253 / Auto-skipped: 0


mssql_mnesia_24 / odbc_mssql_mnesia / 634c0cc
Reports root/ big
OK: 3095 / Failed: 0 / User-skipped: 248 / Auto-skipped: 0


riak_mnesia_24 / riak_mnesia / 634c0cc
Reports root/ big
OK: 1690 / Failed: 0 / User-skipped: 364 / Auto-skipped: 0

- Update dependency processing and resolution
- Update type specs, improving reuse
Do not maintain tests for lists and maps as this state is temporary.
@chrzaszcz chrzaszcz marked this pull request as ready for review February 22, 2022 12:40
@mongoose-im
Copy link
Collaborator

mongoose-im commented Feb 22, 2022

small_tests_24 / small_tests / 7b99d7b
Reports root / small


small_tests_23 / small_tests / 7b99d7b
Reports root / small


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 7b99d7b
Reports root/ big
OK: 2721 / Failed: 0 / User-skipped: 239 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 7b99d7b
Reports root/ big
OK: 2721 / Failed: 0 / User-skipped: 239 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 7b99d7b
Reports root/ big
OK: 2721 / Failed: 0 / User-skipped: 239 / Auto-skipped: 0


dynamic_domains_mysql_redis_24 / mysql_redis / 7b99d7b
Reports root/ big
OK: 2704 / Failed: 0 / User-skipped: 256 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / 7b99d7b
Reports root/ big
OK: 1510 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0


ldap_mnesia_23 / ldap_mnesia / 7b99d7b
Reports root/ big
OK: 1510 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0


internal_mnesia_24 / internal_mnesia / 7b99d7b
Reports root/ big
OK: 1551 / Failed: 0 / User-skipped: 349 / Auto-skipped: 0


pgsql_mnesia_23 / pgsql_mnesia / 7b99d7b
Reports root/ big
OK: 3095 / Failed: 0 / User-skipped: 248 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / 7b99d7b
Reports root/ big
OK: 3095 / Failed: 0 / User-skipped: 248 / Auto-skipped: 0


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 7b99d7b
Reports root/ big
OK: 1685 / Failed: 0 / User-skipped: 369 / Auto-skipped: 0


mssql_mnesia_24 / odbc_mssql_mnesia / 7b99d7b
Reports root/ big
OK: 3095 / Failed: 0 / User-skipped: 248 / Auto-skipped: 0


mysql_redis_24 / mysql_redis / 7b99d7b
Reports root/ big
OK: 3090 / Failed: 0 / User-skipped: 253 / Auto-skipped: 0


riak_mnesia_24 / riak_mnesia / 7b99d7b
Reports root/ big
OK: 1690 / Failed: 0 / User-skipped: 364 / Auto-skipped: 0

@@ -536,13 +536,13 @@ config_metrics(HostType) ->

muclight_dep(List) ->
case lists:member(muclight, List) of
true -> [{mod_muc_light, hard}];
true -> [{mod_muc_light, [], hard}];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I understand - this place, and all these similar ones could be a #{} already?

Copy link
Member Author

@chrzaszcz chrzaszcz Feb 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, if a module still has its options in a list, it should be [], that's the whole point of making this explicit.
When mod_muc_light opts get converted to a map, this should be changed to #{}. Same applies to other modules. Otherwise a module expecting a list would get a map, and it could potentially crash.

@gustawlippa gustawlippa merged commit 7c3eb5f into master Feb 23, 2022
@gustawlippa gustawlippa deleted the module-deps-with-maps branch February 23, 2022 09:07
@Premwoik Premwoik added this to the 5.1.0 milestone May 25, 2022
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.

4 participants