fix: Prevent concurrent updates to git repositories #177
Conversation
eplightning
force-pushed
the
git-concurrency
branch
from
7e291d1
to
d68872b
Compare
Codecov Report
@@ Coverage Diff @@
## master #177 +/- ##
==========================================
- Coverage 68.88% 68.37% -0.51%
==========================================
Files 19 19
Lines 1385 1404 +19
==========================================
+ Hits 954 960 +6
- Misses 346 358 +12
- Partials 85 86 +1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Hey, awesome @eplightning - thanks a lot!
I have just a minor comment (please see below), and also would like to request whether you can put some comments to the new public methods and types that you introduced.
Regarding unit tests, I think we're fine without having those yet.
pkg/argocd/update.go
Outdated
| @@ -367,6 +392,16 @@ func getWriteBackConfig(app *v1alpha1.Application, kubeClient *kube.KubernetesCl | |||
| return wbc, nil | |||
| } | |||
|
|
|||
| func commitChangesLocked(app *v1alpha1.Application, wbc *WriteBackConfig, state *SyncIterationState) error { | |||
| if wbc.Method == WriteBackGit { | |||
There was a problem hiding this comment.
I think we could move that decision to a method within the WriteBackConfig used, e.g.
func (wbc *WriteBackConfig) RequiresLocking() bool {
switch wbc.Method {
case WriteBackGit:
return true
default:
return false
}
}and then use
if wbc.RequiresLocking() {
...
}This would probably make it easier to integrate further write-back methods that might need locking as well.
There was a problem hiding this comment.
Yeah I agree, this makes code bit cleaner too.
Integrated this change into this PR and also added comments for new public type and methods.
* Add method docs and RequiresLocking method
eplightning
force-pushed
the
git-concurrency
branch
from
d68872b
to
1354f0f
Compare
* Add method docs and RequiresLocking method
As requested, here's PR for my attempt at fixing issue with concurrent
git pushoperations.I cleaned it up a little bit and re-ran
go mod tidy(I don't think it changed anything though). As always I'm open to changes, I didn't have chance to write much Go code lately.Fixes #175