fix: if Mirrors repo no content is fetched, updated time should not b… #3551
Conversation
tboerger
added
the
lgtm/need 1
| @@ -264,6 +264,21 @@ func SyncMirrors() { | |||
| log.Error(4, "UpdateMirror [%s]: %v", repoID, err) | |||
| continue | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
But line 263 has updated all the columns of the records.
There was a problem hiding this comment.
yes. It only update the NextUpdateUnix value. Right?
There was a problem hiding this comment.
but we need to update the updated_unix field of repo table.
There was a problem hiding this comment.
@appleboy. No. It should only update NextUpdateUnix maybe I don't know. But in fact it will update all the columns included updated_unix. So that your PR will update the record twice.
There was a problem hiding this comment.
@lunny Line 263 just update the mirror table and this PR update updated_unix field on the other table repository
There was a problem hiding this comment.
Oh, yes you are right. And maybe you can also refactor there to use one DB connection but not get connection from cache.
Codecov Report
@@ Coverage Diff @@
## master #3551 +/- ##
==========================================
- Coverage 35.78% 35.77% -0.01%
==========================================
Files 285 285
Lines 40841 40850 +9
==========================================
Hits 14615 14615
- Misses 24058 24067 +9
Partials 2168 2168
Continue to review full report at Codecov.
|
models/repo_mirror.go
Outdated
| if err != nil { | ||
| log.Error(2, "GetLatestCommitDate [%s]: %v", m.RepoID, err) | ||
| continue | ||
| } else if commitDate.Before(*m.Repo.UpdatedUnix.AsTimePtr()) { |
There was a problem hiding this comment.
- This will not fix already invalid values
- If mirrored repo last commit is deleted with force push, this will not update to new latest commits time
Can't this be checked so to skip only when updated_unix equals commit time?
There was a problem hiding this comment.
@lafriks You are right. Removed the following check.
if commitDate.Before(*m.Repo.UpdatedUnix.AsTimePtr())
tboerger
added
lgtm/done
|
@lafriks Waiting for your response. |
models/repo_mirror.go
Outdated
|
|
||
| // Get latest commit date and compare to current repository updated time, | ||
| // update if latest commit date is newer. | ||
| commitDate, err := git.GetLatestCommitTime(m.Repo.RepoPath()) |
There was a problem hiding this comment.
Comment should also be fixed as there is no check anymore
Signed-off-by: Bo-Yi Wu <[email protected]>
go-gitea#3551) * fix: if Mirrors repo no content is fetched, updated time should not be changed * fix: sync update time from mirror repo. * fix: one single session. * update comment. Signed-off-by: Bo-Yi Wu <[email protected]>
…nged #3551 (#3565) Signed-off-by: Bo-Yi Wu <[email protected]>
maybe fix #3548 and #1329