Klient/machine: start synchronization of stored mounts during KD initialization. - #10358
Conversation
373c175 to
5e7e71c
Compare
Current coverage is 29.86% (diff: 0.00%)@@ master #10358 diff @@
==========================================
Files 93 93
Lines 11068 11075 +7
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
Hits 3308 3308
- Misses 7417 7424 +7
Partials 343 343
|
| } | ||
|
|
||
| func KodingHome() string { | ||
| func KodingConfigHome() string { |
There was a problem hiding this comment.
Why do we need to rename it? The env is KODING_HOME so we can still treat it as KodingHome. Koding home directory stores also kd / klient data and cache, so it's not solely a config dir.
There was a problem hiding this comment.
I would be happy to left this as is. I followed your guide to store cached data under:
$HOME/.cache/koding/*
but if you want I can create cache under:
$HOME/.config/koding/*
path.
There was a problem hiding this comment.
I'm just nitpicking about the naming, as always. Having cache under ~/.cache is cool, nothing to do here. But there's no need to rename KodingHome because of it, that's was my point.
There was a problem hiding this comment.
I reverted this change.
|
|
||
| g.log.Info("Created alias for %s, %s", id, alias) | ||
| for _, id := range noAliases { | ||
| g.log.Warning("Missing alias for %v, regenerating...", id) |
There was a problem hiding this comment.
You use %s at L192 and L195 and here - %v.
| // dynamicClient creates dynamic client functor that is used for mount sync | ||
| // connections. | ||
| func (g *Group) dynamicClient() mountsync.DynamicClientFunc { | ||
| return func(mountID mount.ID) (client.Client, error) { |
There was a problem hiding this comment.
No need for closure here. You can just define this method on Group:
func (g *Group) dynamicClient(id mount.ID) (client.Client, error) {And then use method selector:
syncOpts := mountsync.SyncOpts{
ClientFunc: g.dynamicClient,There was a problem hiding this comment.
I use this approach to in order to make the purpose of this method explicit.
14541e7 to
8492302
Compare
8492302 to
5db6276
Compare
| atomic.AddInt64(&errN, 1) | ||
| g.log.Error("Cannot start synchronization for mount %s: %s", mountID, err) | ||
| } | ||
| }() |
There was a problem hiding this comment.
could you pass mountID, m into go func() ?
There was a problem hiding this comment.
done, you caught a nasty bug 💯
There was a problem hiding this comment.
I extended tests to cover this case.
| } | ||
|
|
||
| // mktree ensures that provided directory is created. | ||
| func mktree(dir string) error { |
Depends on:
#10332Code coverage: 50.3% of statements
How Has This Been Tested?
Unit tests.
Screenshots (if appropriate):
none
Types of changes