Skip to content

Conversation

@louyuting
Copy link
Collaborator

@louyuting louyuting commented Feb 28, 2020

Describe what this PR does / why we need it

Does this pull request fix one issue?

Will define the dynamic data source extension framework.

Describe how you did it

Describe how to verify it

Special notes for reviews

if src == nil {
return nil
}
rs := gjson.ParseBytes(src).Array()
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a little problem: what should I do if the data souce is a file in toml format?

"github.com/tidwall/gjson"
)

func SystemRulesConvert(src []byte) interface{} {
Copy link
Contributor

Choose a reason for hiding this comment

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

[]byte looks a little strange, how about io.Reader?

return errors.New(fmt.Sprintf("Invalid parameters: data:%+v", data))
}
val, ok := data.([]*system.SystemRule)
if !ok {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need compare the data in cache with the new data like the Java design? Or update the value in cache directly?

return ds
}

func (ds *FileDataSource) listen() {
Copy link
Contributor

Choose a reason for hiding this comment

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

As you add two function for type DataSource interface, all the specific datasource should implement it.

//}

func TestNewFileDataSource_SystemRule(t *testing.T) {
ds := FileDataSourceStarter("../../tests/testdata/extension/refreshable_file/SystemRule.json", ext.NewSystemRulesHandle())
Copy link
Contributor

@wenxuwan wenxuwan Mar 4, 2020

Choose a reason for hiding this comment

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

For a better understanding of the frame, I think should declare ds with DataSource type.

@louyuting louyuting marked this pull request as ready for review March 7, 2020 08:22
@louyuting louyuting requested a review from sczyh30 March 7, 2020 08:23
@louyuting louyuting added kind/feature Category issues or PRs related to feature request to-review PRs to review labels Mar 7, 2020
Comment on lines 67 to 71
realSrc, err := json.Marshal(realProperty)
if err!=nil {
logger.Warnf("Fail to marshal property source, err:%+v.", err)
}
isConsistent := h.isPropertyConsistent(realSrc)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can compare the parsed entity directly, rather than convert it to JSON string then compare.

type DefaultPropertyHandler struct {
lastUpdateProperty interface{}

Convert PropertyConvert
Copy link
Member

Choose a reason for hiding this comment

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

Make it private and access it with the getter method?

@louyuting louyuting requested a review from sczyh30 March 9, 2020 13:43
@sczyh30 sczyh30 added the area/data-source Issues or PRs related to data-source extension label Mar 9, 2020
Copy link
Member

@sczyh30 sczyh30 left a comment

Choose a reason for hiding this comment

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

LGTM

@sczyh30 sczyh30 merged commit 1c1d2e1 into alibaba:master Mar 9, 2020
@sczyh30
Copy link
Member

sczyh30 commented Mar 9, 2020

Nice work. Thanks!

@sczyh30 sczyh30 removed the to-review PRs to review label Mar 9, 2020
@sczyh30 sczyh30 linked an issue Mar 10, 2020 that may be closed by this pull request
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 kind/feature Category issues or PRs related to feature request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Basic abstraction for data-source extension

4 participants