faster refresh capabilities #743
Conversation
pkg/commands/refresh.go
Outdated
| @@ -10,6 +16,8 @@ import ( | |||
| cmdutil "github.com/oam-dev/kubevela/pkg/commands/util" | |||
| "github.com/oam-dev/kubevela/pkg/plugins" | |||
| "github.com/oam-dev/kubevela/pkg/utils/system" | |||
|
|
|||
| hashstructure "github.com/mitchellh/hashstructure/v2" | |||
| ) | |||
|
|
|||
| type refreshStatus string | |||
There was a problem hiding this comment.
我见到比较多的 type name 首字母是大写的，请确认一下：）
Codecov Report
@@ Coverage Diff @@
## master #743 +/- ##
==========================================
- Coverage 34.45% 34.40% -0.06%
==========================================
Files 92 92
Lines 8053 8130 +77
==========================================
+ Hits 2775 2797 +22
- Misses 4981 5025 +44
- Partials 297 308 +11
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| timestamp := strconv.FormatInt(time.Now().Unix(), 10) | ||
| tmpDir := filepath.Join(capDir, ".tmp") | ||
| timeFilePath := filepath.Join(tmpDir, ".lastRefresh") | ||
| exist, _ := system.CreateIfNotExist(tmpDir) |
There was a problem hiding this comment.
Should we delete the dir later?
There was a problem hiding this comment.
Since it's in the dir .vela/capabilities, I think it's okay to be deleted along with .vela/capabilities if necessary.
| @@ -397,6 +397,7 @@ func (p PeerHealthConditions) MergePeerWorkloadsConditions(basic *WorkloadHealth | |||
| // copy to keep idempotent | |||
| peerHCs := make(PeerHealthConditions, len(p)) | |||
| copy(peerHCs, p) | |||
| //nolint:makezero | |||
There was a problem hiding this comment.
我懂了 makezero 的含义了，为了更明确，要不要换一个名字，比如 slice-make-zero-length?
There was a problem hiding this comment.
yes... it's an ambiguous name, but makezero is the name of the linter, and I have already commented it in .golangci-lint.yml
pkg/commands/refresh_test.go
Outdated
| . "github.com/onsi/gomega" | ||
| ) | ||
|
|
||
| func TestCheckRefreshInterval(t *testing.T) { |
There was a problem hiding this comment.
please don't use ginkgo for pure unit-test. It's hard to maintain. We prefer ginkgo only when the test need K8s environment.
We prefer table driven test for normal unit-test, you can refer to https://dave.cheney.net/2019/05/07/prefer-table-driven-tests
pkg/commands/refresh.go
Outdated
| @@ -21,14 +29,27 @@ const ( | |||
| deleted refreshStatus = "Deleted" | |||
| ) | |||
|
|
|||
| const ( | |||
| refreshInterval = time.Minute | |||
There was a problem hiding this comment.
please make the default time to be 5min
There was a problem hiding this comment.
got it, shall we add a flag to enforce refresh, just for dev convenience?
There was a problem hiding this comment.
shall we add a flag to enforce refresh, just for dev convenience?
Sure, but users may not be aware of that. That could be very useful for development and demo.
pkg/commands/refresh.go
Outdated
| @@ -10,6 +16,8 @@ import ( | |||
| cmdutil "github.com/oam-dev/kubevela/pkg/commands/util" | |||
| "github.com/oam-dev/kubevela/pkg/plugins" | |||
| "github.com/oam-dev/kubevela/pkg/utils/system" | |||
|
|
|||
| hashstructure "github.com/mitchellh/hashstructure/v2" | |||
| report[unchanged] = append(report[unchanged], newCap) | ||
| break | ||
| } | ||
| } | ||
| if types.EqualCapability(oldCap, newCap) { |
There was a problem hiding this comment.
do we still need this when we have already use cachedHash ?
There was a problem hiding this comment.
Yes, I think we can remove it. It seems unlikely to delete the cache file by someone.
There was a problem hiding this comment.
After reconsideration, I think it's okay to keep this part in case of cached hash is missing.
captainroy-hy
force-pushed
the
faster-refresh
branch
3 times, most recently
from
b3a219e
to
c6216a1
Compare
|
please resolve conflicts |
Signed-off-by: roywang <seiwy2010@gmail.com>
captainroy-hy
force-pushed
the
faster-refresh
branch
from
c6216a1
to
a961b00
Compare
to fix #629
Signed-off-by: roywang seiwy2010@gmail.com