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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(mongodb): add configurable option to override legacy protocol usage #11429

Merged
merged 1 commit into from Aug 16, 2023

Conversation

thalesmg
Copy link
Contributor

@thalesmg thalesmg commented Aug 10, 2023

Fixes https://emqx.atlassian.net/browse/EMQX-10750

Fixes #11428

See emqx/mongodb-erlang#39

Summary

馃 Generated by Copilot at af90f82

Added support for different MongoDB protocol versions in the emqx_mongodb and emqx_bridge_mongodb applications. Updated the dependency, test case, configuration, and i18n files accordingly. Bumped the versions of the affected applications.

PR Checklist

Please convert it to a draft if any of the following conditions are not met. Reviewers may skip over until all the items are checked:

  • Added tests for the changes
  • [na] Added property-based tests for code which performs user input validation
  • Changed lines covered in coverage report
  • Change log has been added to changes/(ce|ee)/(feat|perf|fix)-<PR-id>.en.md files
  • For internal contributor: there is a jira ticket to track this change
  • [na] Created PR to emqx-docs if documentation update is required, or link to a follow-up jira ticket
  • Schema changes are backward compatible

Checklist for CI (.github/workflows) changes

  • [na] If changed package build workflow, pass this action (manual trigger)
  • [na] Change log has been added to changes/ dir for user-facing artifacts update

@thalesmg thalesmg closed this Aug 14, 2023
@thalesmg thalesmg reopened this Aug 14, 2023
@thalesmg thalesmg marked this pull request as ready for review August 14, 2023 13:49
rel/i18n/emqx_mongodb.hocon Outdated Show resolved Hide resolved
ieQu1
ieQu1 previously approved these changes Aug 14, 2023
Comment on lines +523 to +533
{ok, _} = create_bridge(Config, #{<<"use_legacy_protocol">> => <<"true">>}),
?retry(
_Interval0 = 200,
_NAttempts0 = 20,
?assertMatch({ok, connected}, emqx_resource_manager:health_check(ResourceID))
),
WorkerPids0 = get_worker_pids(Config),
Expected0 = maps:from_keys(WorkerPids0, true),
LegacyOptions0 = maps:from_list([{Pid, mc_utils:use_legacy_protocol(Pid)} || Pid <- WorkerPids0]),
?assertEqual(Expected0, LegacyOptions0),
{ok, _} = delete_bridge(Config),
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: maybe put it in a fun?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's only used here twice, so I don't think it's worth doing it at this point. If it starts getting used more often, it's a good idea.

@zmstone
Copy link
Member

zmstone commented Aug 15, 2023

is it still needed after the client is fixed?

@zmstone
Copy link
Member

zmstone commented Aug 15, 2023

bump mongo client version ?

@thalesmg
Copy link
Contributor Author

is it still needed after the client is fixed?

I think it is better to have the option to customize it if needed, even if the auto mechanism is improved.

bump mongo client version ?

It is bumped:

https://github.com/emqx/emqx/pull/11429/files#diff-f6aee252daac224fc006c3c07b8dfe64848a155b6150153d00678a3a273894bcR6

@thalesmg thalesmg merged commit 82f27ea into emqx:master Aug 16, 2023
128 checks passed
@thalesmg thalesmg deleted the mongodb-legacy-opt-20230810 branch August 16, 2023 12:55
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.

None yet

6 participants