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

Improve the default JSON rule parsers in ext/datasource package #191

Closed
sczyh30 opened this issue Jul 27, 2020 · 3 comments · Fixed by #205
Closed

Improve the default JSON rule parsers in ext/datasource package #191

sczyh30 opened this issue Jul 27, 2020 · 3 comments · Fixed by #205
Assignees
Labels
area/data-source Issues or PRs related to data-source extension good first issue Good for newcomers kind/enhancement Category issues or PRs related to enhancement
Milestone

Comments

@sczyh30
Copy link
Member

sczyh30 commented Jul 27, 2020

Issue Description

Type: improvement

Describe what feature you want

The default JSON rule parsers in ext/datasource package could be improved. In current implementation, even if the JSON itself is not an array of flow rule entities, empty FlowRule might be generated:

rules := make([]*flow.FlowRule, 0)
result := gjson.ParseBytes(src)
for _, r := range result.Array() {
flowRule := &flow.FlowRule{
Resource: r.Get("resource").String(),
LimitOrigin: r.Get("limitOrigin").String(),
MetricType: flow.MetricType(r.Get("metricType").Int()),
Count: r.Get("count").Float(),
RelationStrategy: flow.RelationStrategy(r.Get("relationStrategy").Int()),
ControlBehavior: flow.ControlBehavior(r.Get("controlBehavior").Int()),
RefResource: r.Get("refResource").String(),
WarmUpPeriodSec: uint32(r.Get("warmUpPeriodSec").Int()),
MaxQueueingTimeMs: uint32(r.Get("maxQueueingTimeMs").Int()),
ClusterMode: r.Get("clusterMode").Bool(),
ClusterConfig: flow.ClusterRuleConfig{
ThresholdType: flow.ClusterThresholdMode(r.Get("clusterConfig.thresholdType").Int()),
},
}
rules = append(rules, flowRule)
}

Though the empty entity will be filtered out when pumping it to downstream consumers, the creation of the "invalid" entity seems useless. In addition, it might be better to leverage the unified quotes of the fields (aka. json:"xxx") to parse the object instead of writing the logic manually.

The naming could be improved as well. For example: FlowRulesJsonConverter can be FlowRuleJsonArrayParser or other better.

Additional context

Add any other context or screenshots about the feature request here.

@sczyh30 sczyh30 added kind/enhancement Category issues or PRs related to enhancement area/data-source Issues or PRs related to data-source extension labels Jul 27, 2020
@sczyh30 sczyh30 added this to the 0.6.0 milestone Jul 27, 2020
@binbin0325
Copy link
Collaborator

I'll try to work it out

@sczyh30
Copy link
Member Author

sczyh30 commented Jul 30, 2020

cc @louyuting Any suggestions?

@sczyh30 sczyh30 added the good first issue Good for newcomers label Jul 30, 2020
@louyuting
Copy link
Collaborator

In current implementation, even if the JSON itself is not an array of flow rule entities, empty FlowRule might be generated:

Maybe we could check the validity after parsing entity.

Naming improvement make sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/data-source Issues or PRs related to data-source extension good first issue Good for newcomers kind/enhancement Category issues or PRs related to enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants