feat: Use shallow clone to speed up performance #830
Conversation
jnovick
commented
Aug 13, 2024
jnovick
commented
- Only git fetch the one target revision that we need
- This does not utilize the history so there is no reason to clone it
- For large repos, this will speed up performance significantly
jnovick
requested review from
jannfis,
chengfang and
pasha-codefresh
as code owners
jnovick
force-pushed
the
shallow-clone
branch
2 times, most recently
from
cba9345 to
fe16b69
Compare
ext/git/client.go
Outdated
| @@ -384,6 +385,40 @@ func (m *nativeGitClient) Fetch(revision string) error { | |||
| return err | |||
| } | |||
|
|
|||
| func (m *nativeGitClient) shallowFetch(revision string, depth int) error { | |||
There was a problem hiding this comment.
Can you please move both functions to a separate file and only leave the change to the interface in this one? Reason is, that we copy over the Git code from Argo CD on every release. If this code stays in an file that exists in Argo CD too, it makes it pretty complicated to maintain. Thanks!
There was a problem hiding this comment.
No problem, moved it. I chose not to update the existing fetch function since I saw it was copied in, but I wasn't sure where to put new logic, but I found it now.
You have made me wonder if I can contribute to ArgoCD, but updating it to use shallow clones as well. I will be investigating that later.
Thanks, I appreciate the help :D
There was a problem hiding this comment.
I think we discussed shallow cloning with Argo CD in the past a couple of times and came to no final conclusion yet. It's a pretty invasive change due to repository caching, history requirements for rollbacks and information etc.
For Image Updater however, I think there is no issue with shallow cloning and I believe it's beneficial.
* Only git fetch the one target revision that we need * This does not utilize the history so there is no reason to clone it * For large repos, this will speed up performance significantly Signed-off-by: Joshua Novick <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #830 +/- ##
=======================================
Coverage 73.53% 73.53%
=======================================
Files 31 31
Lines 3140 3140
=======================================
Hits 2309 2309
Misses 695 695
Partials 136 136 ☔ View full report in Codecov by Sentry. |
jannfis
changed the title
Signed-off-by: Joshua Novick <[email protected]> Signed-off-by: Tchoupinax <[email protected]>
