Skip to content

[Improvement-16440][Alert] Improvement alert plugin get name#16441

Closed
sdhzwc wants to merge 1 commit intoapache:devfrom
sdhzwc:dev
Closed

[Improvement-16440][Alert] Improvement alert plugin get name#16441
sdhzwc wants to merge 1 commit intoapache:devfrom
sdhzwc:dev

Conversation

@sdhzwc
Copy link
Contributor

@sdhzwc sdhzwc commented Aug 13, 2024

close #16440

Purpose of the pull request

Brief change log

Verify this pull request

This pull request is code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(or)

Pull Request Notice

Pull Request Notice

If your pull request contain incompatible change, you should also add it to docs/docs/en/guide/upgrede/incompatible.md

Copy link
Member

@SbloodyS SbloodyS left a comment

Choose a reason for hiding this comment

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

I don't think this optimization is necessary at this stage, because these names will only be initialized once and there is no need to reuse them.

Comment on lines +29 to +42
public enum AlertPluginType {

DINGTALK(0, "DingTalk", "ding talk"),
EMAIL(1, "Email", "email"),
FEISHU(2, "Feishu", "fei shu"),
HTTP(3, "Http", "http"),
PAGERDUTY(4, "PagerDuty", "pager duty"),
PROMETHEUS(5, "Prometheus", "prometheus alert manager"),
SCRIPT(6, "Script", "script"),
SLACK(7, "Slack", "slack"),
TELEGRAM(8, "Telegram", "telegram"),
WEBEXTEAMS(9, "WebexTeams", "webex teams"),
WECHAT(10, "WeChat", "we chat"),
ALIYUNVOICE(11, "AliyunVoice", "aliyun voice");
Copy link
Member

@ruanwenjun ruanwenjun Aug 19, 2024

Choose a reason for hiding this comment

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

It's better don't add a enum here, otherwise, once we add a new plugin we should modify the enum.
We should make the plugin doesn't rely the other module.

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 better don't add a enum here, otherwise, once we add a new plugin we should modify the enum. We should make the plugin doesn't rely the other module.
Referring to the DbType enumeration, isn't a new data source maintained in the same way?
image

@SbloodyS SbloodyS closed this Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Improvement][Alert] Improvement alert plugin get name

3 participants