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

feat: add connector schema scaffold and break out Kafka conector #11581

Conversation

kjellwinblad
Copy link
Contributor

@kjellwinblad kjellwinblad commented Sep 8, 2023

This work in progress draft PR is the beginning of an effort to split bridges into a connector part and a bridge part.

Several bridges should be able to share a connector pool defined by a single connector. The connectors should be possible to enable and disable similar to how one can disable and enable bridges. There should also be an API for checking the status of a connector and for add/edit/delete connectors similar to the current bridge API.

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

NOTE:

This PR still requires a lot of work and is just created to increase transparency while doing the work.

Summary

🤖 Generated by Copilot at 02cd5d0

This pull request adds hocon schema support for the kafka connector configuration in the enterprise edition of emqx. It refactors and documents the connector_config fields in emqx_bridge_kafka.erl and adds new schema modules in emqx_conf and emqx_connector 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
  • 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
  • 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

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

@kjellwinblad kjellwinblad force-pushed the kjell/break_out_connection_settings_from_bridge/EMQX-10805/EMQX-10770 branch from 5c5baac to 05c4348 Compare September 22, 2023 15:16
@kjellwinblad kjellwinblad force-pushed the kjell/break_out_connection_settings_from_bridge/EMQX-10805/EMQX-10770 branch from 765b2d3 to 0953a8a Compare September 29, 2023 16:25
apps/emqx_resource/src/emqx_resource.erl Outdated Show resolved Hide resolved
Comment on lines +515 to +553
false ->
{error,
<<<<"on_add_channel callback function not available for connector with resource id ">>/binary,
ResId/binary>>}
Copy link
Member

Choose a reason for hiding this comment

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

if this happens, isn't it a bug?
we should perhaps crash it or perform higher level sanity check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, we could crash here. Will fix

apps/emqx_resource/src/emqx_resource.erl Outdated Show resolved Hide resolved
Comment on lines 66 to 67
%% TODO: rename this to `kafka_producer' after alias support is added
%% to hocon; keeping this as just `kafka' for backwards compatibility.
Copy link
Member

Choose a reason for hiding this comment

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

now it's the time for this TODO?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is probably a good time. We can try to do this before the merge.

@kjellwinblad kjellwinblad force-pushed the kjell/break_out_connection_settings_from_bridge/EMQX-10805/EMQX-10770 branch 3 times, most recently from 7fb736e to 4f04afb Compare October 17, 2023 16:24
@kjellwinblad kjellwinblad changed the base branch from master to release-53 October 17, 2023 16:26
@kjellwinblad kjellwinblad reopened this Oct 17, 2023
@kjellwinblad kjellwinblad force-pushed the kjell/break_out_connection_settings_from_bridge/EMQX-10805/EMQX-10770 branch from b88f282 to e3260a5 Compare October 17, 2023 18:52
@kjellwinblad kjellwinblad force-pushed the kjell/break_out_connection_settings_from_bridge/EMQX-10805/EMQX-10770 branch from f843b52 to a2cb00b Compare October 25, 2023 07:41
@kjellwinblad kjellwinblad reopened this Oct 25, 2023
@id id force-pushed the kjell/break_out_connection_settings_from_bridge/EMQX-10805/EMQX-10770 branch from a70de06 to 26f0cb8 Compare October 25, 2023 18:15
@zmstone zmstone force-pushed the kjell/break_out_connection_settings_from_bridge/EMQX-10805/EMQX-10770 branch from 32f01cd to 1ce7c2e Compare October 27, 2023 07:29
kjellwinblad and others added 11 commits October 27, 2023 11:53
This commit is the beginning of an effort to split bridges into a
connector part and a bridge part.

Several bridges should be able to share a connector pool defined by a
single connector. The connectors should be possible to enable and
disable similar to how one can disable and enable bridges. There should
also be an API for checking the status of a connector and for
add/edit/delete connectors similar to the current bridge API.

Issues:
https://emqx.atlassian.net/browse/EMQX-10805
* Most Bridge V1 HTTP API calls are now compatible with Bridge V2
* Local topics works for Bridge V2 now
* A lot of work on trying to get the old Kafka producer test suite
  to work after the refactorings
@zmstone zmstone force-pushed the kjell/break_out_connection_settings_from_bridge/EMQX-10805/EMQX-10770 branch from 85d0876 to 5f17a8f Compare October 27, 2023 09:53
thalesmg and others added 18 commits October 27, 2023 08:23
Most operation for the bridge V1 HTTP API compatibility layer are now
working. This has been tested by creating/deleting/updating Kafka bridge
through HTTP API, sending message to it, and resetting and checking
metrics.

The start, stop, restart, enable, disable operations still need to be
fixed.
* Make sure that a rule that refer to a bridge that has been converted to
a bridge V2 bridge gets its type converted if needed.
* Add test case for sending message to a Bridge V2 through a rule
@id id mentioned this pull request Oct 30, 2023
9 tasks
@kjellwinblad
Copy link
Contributor Author

Closing this since @id has squashed and merged to release-53

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