Fix initial_sync for existing watch files#13728
Fix initial_sync for existing watch files#13728trinhchien wants to merge 1 commit intodocker:mainfrom
Conversation
glours
left a comment
glours
left a comment
There was a problem hiding this comment.
Hey @trinhchien
Thanks for the contribution, there is few details to adapt in your PR. Can you take a look?
pkg/compose/watch.go
Outdated
There was a problem hiding this comment.
With the change this function is now deadcode and should be removed
pkg/compose/watch.go
Outdated
There was a problem hiding this comment.
With the changes this is no longer accurate
pkg/compose/watch.go
Outdated
There was a problem hiding this comment.
ctx and project aren't used anymore
| func (s *composeService) initialSyncFiles(service types.ServiceConfig, trigger types.Trigger, ignore watch.PathMatcher) ([]*sync.PathMapping, error) { |
pkg/compose/watch_test.go
Outdated
| apiClient := mocks.NewMockAPIClient(mockCtrl) | ||
| cli.EXPECT().Client().Return(apiClient).AnyTimes() |
There was a problem hiding this comment.
No more reference to the api client, so we don't need the mock
pkg/compose/watch_test.go
Outdated
| apiClient := mocks.NewMockAPIClient(mockCtrl) | ||
| cli.EXPECT().Client().Return(apiClient).AnyTimes() |
pkg/compose/watch_test.go
Outdated
| hostFile := filepath.Join(dir, "test.txt") | ||
| assert.NilError(t, os.WriteFile(hostFile, []byte("hello"), 0o644)) | ||
|
|
||
| fileTime := time.Date(2026, 4, 1, 10, 0, 0, 0, time.UTC) |
There was a problem hiding this comment.
| fileTime := time.Date(2026, 4, 1, 10, 0, 0, 0, time.UTC) | |
| fileTime := time.Now().Add(-24*time.Hour) |
pkg/compose/watch_test.go
Outdated
| hostFile := filepath.Join(hostDir, "test.txt") | ||
| assert.NilError(t, os.WriteFile(hostFile, []byte("hello"), 0o644)) | ||
|
|
||
| fileTime := time.Date(2026, 4, 1, 10, 0, 0, 0, time.UTC) |
There was a problem hiding this comment.
| fileTime := time.Date(2026, 4, 1, 10, 0, 0, 0, time.UTC) | |
| fileTime := time.Now().Add(-24*time.Hour) |
trinhchien
force-pushed
the
fix/watch-initial-sync-existing-files
branch
from
6bd0f8b to
f523966
Compare
|
Thanks for the review — I addressed the cleanup comments, simplified the tests as suggested, and updated the commit with a proper DCO sign-off. |
glours
left a comment
glours
left a comment
There was a problem hiding this comment.
You need to sign-off your commit for legal reasons otherwise I won't be able to merge your contribution
pkg/compose/watch.go
Outdated
| // Syncs files from develop.watch.path if thy have been modified after the image has been created | ||
| // Syncs files from develop.watch.path, ignoring bind-mounted and excluded paths. | ||
| // | ||
| //nolint:gocyclo |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
trinhchien
force-pushed
the
fix/watch-initial-sync-existing-files
branch
from
f523966 to
792f8be
Compare
Signed-off-by: trinhchien <trinhdacchien1598@gmail.com>
|
Thanks — I added a small regression test to cover the initialSync path as well, and updated the branch. |
trinhchien
force-pushed
the
fix/watch-initial-sync-existing-files
branch
from
792f8be to
275cad2
Compare
2 participants
Summary
initial_synccopy existing files regardless of image creation timeRepro
Fixes #13725. Before this change,
initial_sync: truecould skip files that already existed on the host if their mtime was older than the service image creation time. Change-triggered syncs still worked, but the initial startup sync could be empty.Testing
go test ./pkg/compose -run TestInitialSyncFiles_ -vAI usage
Used AI assistance to help analyze the repro, shape the initial patch, and draft the tests. I reviewed and validated the final code and test results manually.