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: add runtime manager for bare-metal cluster #169

Merged
merged 6 commits into from
Oct 26, 2023

Conversation

shawnh2
Copy link
Contributor

@shawnh2 shawnh2 commented Oct 12, 2023

  • implement a new simple runtime manager for bare-metal to manage all the using dirs and paths
  • rm the deployer test for bare-metal

all the changes that has been made in this PR, is for the conveniency of later code refactors and migrations.

Signed-off-by: sh2 <shawnhxh@outlook.com>
@shawnh2 shawnh2 requested a review from zyy17 October 12, 2023 08:51
Signed-off-by: sh2 <shawnhxh@outlook.com>
@zyy17
Copy link
Collaborator

zyy17 commented Oct 16, 2023

@shawnh2 Can we put the metadata management(runtime_manager.go) in pkg/metadata by adding the new API in the Manager interface? I think it's better to keep all the metadata rw in a single instance, it also will make it easy to maintain because all the metadata operations are in one place.

@shawnh2
Copy link
Contributor Author

shawnh2 commented Oct 20, 2023

@shawnh2 Can we put the metadata management(runtime_manager.go) in pkg/metadata by adding the new API in the Manager interface? I think it's better to keep all the metadata rw in a single instance, it also will make it easy to maintain because all the metadata operations are in one place.

sounds great, let me refactor it!

@shawnh2
Copy link
Contributor Author

shawnh2 commented Oct 20, 2023

the failed test in https://github.com/GreptimeTeam/gtctl/actions/runs/6573826229/job/17857729564 will also be fixed in this PR

Signed-off-by: sh2 <shawnhxh@outlook.com>
Signed-off-by: sh2 <shawnhxh@outlook.com>
Signed-off-by: sh2 <shawnhxh@outlook.com>
@codecov-commenter
Copy link

codecov-commenter commented Oct 23, 2023

Codecov Report

Attention: 174 lines in your changes are missing coverage. Please review.

Comparison is base (076389d) 35.80% compared to head (df4655a) 37.67%.
Report is 1 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #169      +/-   ##
===========================================
+ Coverage    35.80%   37.67%   +1.87%     
===========================================
  Files           12       14       +2     
  Lines         1430     1189     -241     
===========================================
- Hits           512      448      -64     
+ Misses         814      646     -168     
+ Partials       104       95       -9     
Files Coverage Δ
pkg/cluster/baremetal/validate.go 84.21% <84.21%> (ø)
pkg/cluster/baremetal/not_implemented.go 0.00% <0.00%> (ø)
pkg/metadata/manager.go 74.15% <67.27%> (-11.14%) ⬇️
pkg/cluster/baremetal/config.go 0.00% <0.00%> (ø)
pkg/cluster/baremetal/cluster.go 0.00% <0.00%> (ø)
pkg/cluster/baremetal/get.go 12.74% <12.74%> (ø)

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pkg/metadata/manager.go Outdated Show resolved Hide resolved
pkg/metadata/manager_test.go Outdated Show resolved Hide resolved
pkg/metadata/manager.go Outdated Show resolved Hide resolved
Signed-off-by: sh2 <shawnhxh@outlook.com>
@shawnh2 shawnh2 requested a review from zyy17 October 24, 2023 06:41
Copy link
Collaborator

@zyy17 zyy17 left a comment

Choose a reason for hiding this comment

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

LGTM

@zyy17 zyy17 merged commit 5e2fb15 into GreptimeTeam:develop Oct 26, 2023
6 checks passed
@shawnh2 shawnh2 deleted the refactor/bm-runtime-manager branch October 26, 2023 03:43
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.

3 participants