feat(desktop): synchronized file share integration#11614
Conversation
milas
requested review from
glours,
jhrotko and
ndeloof
and removed request for
a team
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11614 +/- ##
==========================================
- Coverage 57.58% 55.65% -1.93%
==========================================
Files 140 142 +2
Lines 11805 12241 +436
==========================================
+ Hits 6798 6813 +15
- Misses 4322 4740 +418
- Partials 685 688 +3 ☔ View full report in Codecov by Sentry. |
ndeloof
left a comment
ndeloof
left a comment
There was a problem hiding this comment.
LGTM
My understanding was we wanted file share to be used only for watch sync, not for all bind mounts, maybe I missed some discussion :)
xenoscopic
left a comment
xenoscopic
left a comment
There was a problem hiding this comment.
LGTM, my only real concern is the project label values, but maybe Compose's normalization is sufficient? If we can add label validation to the backend, then it's a little less worrying that we might ingest an invalid value.
| if share.Labels["com.docker.compose.project"] == projectName { | ||
| toRemove = append(toRemove, share) | ||
| } |
There was a problem hiding this comment.
We could extend the file shares API with a mechanism to remove these using a label selector. I'm not sure if that would make things more robust; it might make status updates more difficult.
| } | ||
|
|
||
| func (c *Client) ListFileShares(ctx context.Context) ([]FileShareSession, error) { | ||
| req, err := http.NewRequestWithContext(ctx, http.MethodGet, backendURL("/mutagen/file-shares"), http.NoBody) |
There was a problem hiding this comment.
Maybe we should add some comments in the backend that these URL patterns need to stay fixed. I guess we could add some sort of service discovery mechanism, but that seems like overkill.
| req := CreateFileShareRequest{ | ||
| HostPath: path, | ||
| Labels: map[string]string{ | ||
| "com.docker.compose.project": m.projectName, |
There was a problem hiding this comment.
We may need to sanitize m.projectName to comply with Mutagen's (i.e. Kubernetes') label value syntax (maybe it already is, I'm not sure). I don't think it will affect the loop-based approach you have above, but it will break LabelSelector-based selection in the Mutagen synchronization manager (possibly/probably for other sessions too).
Initial integration between Docker Desktop's "Synchronized File Shares" functionality and Compose. When running under Docker Desktop, File Shares will be created/used for any bind mount paths. (There's a missing feature check that still needs to be added, but ALL desktop functionality is currently gated behind the `COMPOSE_EXPERIMENTAL_DESKTOP` env var.) Compose will wait for the File Share to reach steady state, i.e. having completed initial synchronization. The progress is shown, similar to how layer pulls for images are displayed. Signed-off-by: Milas Bowman <[email protected]>
4 participants
What I did
Initial integration between Docker Desktop's "Synchronized File Shares" functionality and Compose.
When running under Docker Desktop, File Shares will be created/used for any bind mount paths. (There's a missing feature check that still needs to be added, but ALL desktop functionality is currently gated behind the
COMPOSE_EXPERIMENTAL_DESKTOPenv var.)Compose will wait for the File Share to reach steady state, i.e. having completed initial synchronization. The progress is shown, similar to how layer pulls for images are displayed.
Related issue
JIRA:
(not mandatory) A picture of a cute animal, if possible in relation to what you did
