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 render package #145

Merged
merged 13 commits into from
Apr 24, 2021
Merged

Refactor render package #145

merged 13 commits into from
Apr 24, 2021

Conversation

Felixoid
Copy link
Collaborator

@Felixoid Felixoid commented Apr 16, 2021

I am kindly sorry for such huge changes, but here is only two complex commits, all other things should be easy to review.

So, here are:

  • Refactored data.Targets + tests for Targets.selectDataTable
  • Optimized external data body building for external-data (TODO: put it to clickhouse/external_data.go)
  • Refactored data.carbonlink and added test there
  • Updated github.com/stretchr/testify to v1.7.0 for Mock
  • Redone data.Reply to data.query:
    • Merged the aggregated and unaggregated pipelines together
    • Split it into functions
    • Added tests for it
  • Reworked data.Data:
    • Moved async logic for response parsing there
    • Merge unaggregated and aggregated parsers
    • Improved tests
  • Added -race for tests
  • Added tests for config.ProcessDataTables

@Felixoid Felixoid force-pushed the refactor_render_data branch 2 times, most recently from 268732e to 092e7cd Compare April 16, 2021 23:42
@Felixoid
Copy link
Collaborator Author

Next move - refactoring data parsing, get rid of channels, flattening the query generating.

@Felixoid Felixoid force-pushed the refactor_render_data branch 4 times, most recently from 71ff0f7 to 6e1a4f6 Compare April 21, 2021 13:14
@Felixoid Felixoid force-pushed the refactor_render_data branch 6 times, most recently from 1849091 to 1d7cbc3 Compare April 22, 2021 20:42
Felixoid added 11 commits April 22, 2021 23:00
- MultiFetchData:
  - `FetchDataPoints` is now a method of `MultiFetchRequest`
  - It returns `[]CHResponse` aka `CHResponses`, not `Reply`
  - Reorganize it a little bit
  - MaxDataPoints is checked and reset here
- Type `Reply` is renamed to private `query`
  - `commonStep` is created in `newQuery`
  - it contains all necessary configs as fields
- Data:
  - Remove rollupObj
  - Make `EmptyData` a private
- point.Points: Change steps map to reduce memory consumption
- where: use int64 timestamps in DateBetween as in other functions
= render/data:
== data/data.go:
- data.Data: rename Aliases to AM as in other types
- create data.data to not propagate internals to outside
- put logic for async parallel queries processing into data.data
- merge parseAggregatedResponse and parseUnaggregatedResponse in parseResponse:
  aggregated query does not uses nullable anymore, so only one function could
  be used
- split tests for parsers and async functions
== data/query.go:
- rework aggregated query to get rid of nullables
- add conditions type that implies TimeFrame and Targets
- decomposite getDataAggregated and getDataUnaggregated methods by
  implementing logic with contitions methods
== data/targets.go:
- use TimeFrame instead of until/from values
@Felixoid Felixoid force-pushed the refactor_render_data branch from 291e5b5 to d64b9d4 Compare April 22, 2021 21:01
@Felixoid Felixoid requested a review from Civil April 23, 2021 09:17
@Felixoid Felixoid force-pushed the refactor_render_data branch from c856dbe to 61b8c70 Compare April 23, 2021 21:23
@Felixoid Felixoid marked this pull request as ready for review April 23, 2021 21:45
@Civil Civil merged commit 860490c into master Apr 24, 2021
@Civil Civil deleted the refactor_render_data branch April 24, 2021 20:31
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.

2 participants