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

Refactor data source hierarchy: spilt into readable and writable data source #124

Merged
merged 4 commits into from
Sep 4, 2018
Merged

Refactor data source hierarchy: spilt into readable and writable data source #124

merged 4 commits into from
Sep 4, 2018

Conversation

sczyh30
Copy link
Member

@sczyh30 sczyh30 commented Sep 3, 2018

Describe what this PR does / why we need it

Spilt data source into readable and writable data source. See #94 for details.

Does this pull request fix one issue?

Closes #94

Describe how you did it

  • Spilt DataSource into two types: ReadableDataSource and WritableDataSource
  • The AbstractDataSource now is read-only.
  • Refactor the file data source for writable implementation (now it's both ReadableDataSource and WritableDataSource)
  • Rename: ConfigParser -> Converter (represents both encoder T -> S and decoder S -> T)
  • Update command handler for modifying rules (code refinement)
  • Update demos of data source
  • Some other refinement

Describe how to verify it

See demo.

- Spilt DataSource into two types: ReadableDataSource and WritableDataSource
- The AbstractDataSource now is read-only
- Refactor the file data source for writable implementation
- Rename: ConfigParser -> Converter (represents both encoder `T -> S` and decoder `S -> T`)
- Some other refinement

Signed-off-by: Eric Zhao <[email protected]>
- Refine code about writable datasource

Signed-off-by: Eric Zhao <[email protected]>
@sczyh30 sczyh30 added the to-review To review label Sep 3, 2018
@sczyh30 sczyh30 self-assigned this Sep 3, 2018
@sczyh30 sczyh30 requested a review from CarpenterLee September 3, 2018 11:31
@codecov-io
Copy link

codecov-io commented Sep 3, 2018

Codecov Report

Merging #124 into master will increase coverage by 0.02%.
The diff coverage is 50%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #124      +/-   ##
============================================
+ Coverage     45.75%   45.77%   +0.02%     
  Complexity      555      555              
============================================
  Files           114      114              
  Lines          3814     3814              
  Branches        531      531              
============================================
+ Hits           1745     1746       +1     
+ Misses         1857     1856       -1     
  Partials        212      212
Impacted Files Coverage Δ Complexity Δ
...inel/datasource/zookeeper/ZookeeperDataSource.java 55.55% <50%> (ø) 4 <0> (ø) ⬇️
...ntinel/slots/statistic/metric/WindowLeapArray.java 68% <0%> (-12%) 5% <0%> (-1%)
...ibaba/csp/sentinel/eagleeye/EagleEyeLogDaemon.java 30.3% <0%> (+6.06%) 6% <0%> (+1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6799da2...46bef74. Read the comment docs.

- Add `close` method in WritableDataSource (to extend AutoCloseable in JDK 1.7 later)
- Separate the writable file data source from original class
- Add a sample to show how to register data sources via Sentinel init mechanism
- Separate a writable data source registry from original handler to make it clear

Signed-off-by: Eric Zhao <[email protected]>
Copy link
Contributor

@CarpenterLee CarpenterLee 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 added this to the 0.2.0 milestone Sep 4, 2018
@sczyh30 sczyh30 merged commit 007cd9d into alibaba:master Sep 4, 2018
@sczyh30 sczyh30 removed the to-review To review label Sep 4, 2018
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.

[Refactor] Divide data source into readable and writable types
3 participants