-
Notifications
You must be signed in to change notification settings - Fork 775
introduce develop
section
#253
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a very promising addition - especially the idea potential idea of Compose better supporting remote filesystems out of the box 😍 I've left a few comments and questions inline to better understand this 💬
development.md
Outdated
|
||
While Compose implementations can support custom values, the specification defines: | ||
- `build`: rebuild service image based on the `build` section and restart the service with updated image | ||
- `sync`: synchronize source files with container content. `path` attribute is REQUIRED to define the mapping |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you elaborate a bit more on how this interacts with host mounts in volumes
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is intentionally unspecified so implementation can be flexible. Most obvious approach is to add an implicit bind mount for sources.
For illustration, a classic nodeJS application uses a COPY directive in Dockerfile to add sources into /app folder. With such a sync directive, docker compose dev
would add a new bind mount so that local sources override this /app folder and get synced inside container, assuming node runtime manages hot-reload.
But this is just a possible implementation, and something more clever could be offered, without the need to change the specification
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for explaining 👍 My concern is that we are potentially adding another way to mount files into the containers to the spec without necessarily drawing a clear distinction for when users should do volumes and when they should be using develop
section instead. Is there a use case that this would cover that volumes are not able to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is still unclear for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did another pass on this PR and left a few comments and suggestions inline 💬 Thank you for iterating on this!
development.md
Outdated
strategy: sync | ||
# get service to reload configuration | ||
signal: SIGHUP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this example include a path for what's being synced? According to the definition, build context would be used as a default but one is not defined here.
development.md
Outdated
signal: SIGHUP | ||
``` | ||
|
||
## Developemnt mode definition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
## Developemnt mode definition | |
## Development mode definition |
typo
development.md
Outdated
#### quiet_period | ||
|
||
If a `quiet_period` is set, Compose Implementations MUST wait at least configured delay before actually refreshing | ||
service image. Using this allows to avoid service image to be re-created many times in a row as multiple files get |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this attribute applies to sync
strategy too, then we should make this paragraph a bit more abstract. Currently, it focuses a lot on rebuilding the image.
development.md
Outdated
|
||
#### trigger | ||
|
||
`trigger` define the actions to take place as changes have been detected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
`trigger` define the actions to take place as changes have been detected. | |
`trigger` defines the actions to take place as changes have been detected. |
typo
development.md
Outdated
is REQUIRED: | ||
|
||
- `build` strategy will rebuild service image based on the `build` section and restart the service with updated image. | ||
- `sync` strategy keep existing service contianers running, but can synchronize source files with container content. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- `sync` strategy keep existing service contianers running, but can synchronize source files with container content. | |
- `sync` strategy keep existing service containers running, but can synchronize source files with container content. |
typo
Hello! I'm not sure if this is the right place to post comments on the develop file-watch feature ; but that's what it says in the docs at https://docs.docker.com/compose/file-watch/ I have a docker-compose file with 12 services in it. For instance I have 2 services that do not mention "x-develop -> watch" in the service, yet when some files get updated, they are rebuilt automatically. Rebuilding indexer-typescript, azfunc-all-v4-5 after changes were detected:
I'd like to be able to use docker-compose alpha watch command and that it does NOT watch apps that do not mention the x-develop option. Or an alternative would be to specify x-develop -> watch and use a pre-defined option : none. When I build an app it writes a lot of files to the dist folder and the docker-compose alpha watch command goes in frenzy rebuilding everything forever using all my hard-disk space. |
@simonmallet github.com/docker/compse would be the right place for this as |
(Assuming this is the right place for this discussion, as per the announcement blog post linking back here...) Related to @EricHripko's comments re: watch.source paths being relative to the build.context, if this is the intention, it would be useful to specify a reference to build.additional_contexts to allow syncing of code outside the main build context. Something like a |
@tr00st see #253 (comment) ;) |
Aah. Apologies, thought that was specifically a bug, hadn't read all the way to the end. I'll try and find the right place to report the dodgy advice for feedback in docs and blog posts ;) |
Automatically update services with file watch. |
Hi @ndeloof I was directed here from the blog post. The blog states that the draft spec consists of actions: "sync" and "rebuild+recreate". There is another action which just as important, and FAR simpler than those: "recreate only". In a CI/CD environment:
I saw you going back and forth on this with some doubters in other threads. But this new watch experiment is a brilliant idea. Maybe consider this use case as the foot in the door - it's simple to explain and justify, and sorely needed. Doing CD with docker is hard, but a watch-restart feature would make it trivial. Please consider giving us a "restart only" action - I would use it in production immediately and toss out half a dozen scripts and tools and kludges! Thanks! |
Just to clarify, you're looking for a watch with a pull and recreate, not just a restart, correct? I know this thread isn't about 3rd party tooling, but just to mention a few ways I've been doing this to make life easier:
|
Yes, you are right, it's a pull + recreate - the equivalent of
Thank you so much for your advice - I'm looking into those options right now. Appreciated! For anyone who needs more options, there's also webhook and webhookd. To eliminate all that stuff for a native and dead-simple docker compose watch feature would be a game changer. Complex CI/CD toolchains would be much shorter and easier to deploy and manage. |
Hi, in the documentation it says:
However, include attribute does not work. Thanks |
@Sevi7 this PR is an early proposal for |
c8ff5bb
to
98583c8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🥳
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
@lonix1 please open a new proposal (issue) for your need. We had to ship a first version of this new section of the spec to move forward and now we're in position to start incremental evolution of it. |
Hello, from what I have read here, this is the place to report bugs and give feedback on the new feautre: I am loving this feature, and it works pretty fine, but I have a problem. When I exit the watch command by using ctrl + c, or by using the command: cannt take exclusive lock for project "my_project": process with PID 11324 is still running What can I do? The only solution I have found is to reset the computer. Docker Compose version v2.22.0-desktop.2 Thanks in ahead for any help :) |
@Avihais12344 this is an implementation issue, please report on https://github.com/docker/compose |
Hi, First thanks for this very nice feature, it is a lot granular and user friendly than bind mounting ;-) I used it and it's simply awsome 👍 I just wanted to make a small feedback about the logs : with As the watch command can't be launched in detached mode like AFAIK, there isn't any detached ou logs flag dedicated to watch command, as for up one. It would be very usefull though :-) Thanks. |
Just my 2 cents, could be the compose up command with a watch flag : Thanks |
Do i always have to have a build section for the watch mode? So for example we have a simple js application and i want to run this one in a container watching file changes for hot reload. If i understand the watch feature correctly i now need a Dockerfile that covers all the stuff i mentioned above and use this one in the compose file. But couldn't it make sense, that i just define the node image in the compose file and the copying is done by the watch section. So instead of this in the compose file: web:
build:
context: .
dockerfile: ./Dockerfile
command: npm start
develop:
watch:
- action: sync
path: .
target: /src where the Dockerfile looks like this:
it could be just this in the compose file: web:
image: node
command: npm start
develop:
watch:
- action: sync
path: .
target: /src Because now i always have to build the image. |
@MarcusElevait the solution you describe would mean application can't run correctly without For your specific scenario, you could use inlined dockerfile: web:
build:
context: .
dockerfile-inline: |
FROM node
COPY . /src
command: npm start
develop:
watch:
- action: sync
path: .
target: /src |
What i've done now is this: web:
image: node
command: npm start
volumes:
- ./:/src the downsides i see currently when using watch instead are:
|
+1 @MarcusElevait |
And |
docker compose watch is always rebuilding image while docker compose up --build is skipping rebuild. |
related: #418 |
I submitted this feedback a few days ago to a Google Docs Form I found, but I just discovered this seems to be the official way, so I'll leave here too: |
Would it be possible to just |
watch configuration is designed to avoid you set a bind mount on your own, so doesn't really make sense |
does it? |
Hello there!
Compose file (partial): services:
django: &django
build:
context: .
dockerfile: ./compose/local/django/Dockerfile
image: local_django
container_name: local_django
depends_on:
- postgres
- redis
env_file:
- path: ./.envs/.local/.django
- path: ./.envs/.local/.postgres
- path: .env
required: false
ports:
- '8001:8000'
command: /start
extra_hosts:
- "host.docker.internal:host-gateway"
develop:
watch:
- action: sync
path: .
target: /app
ignore:
- ./requirements
- action: rebuild
path: requirements The goal here was to rebuild the application when the "requirements" folder content was changed. |
This is my use case for a plain "restart" action. I want to leave the bind in place. Then I don't need to worry about changing users to get write permissions to files that I normally leave read-only in my production environment. When my |
Just found this functionality, and also concerned about the missing plain "restart" action, which will restart container if path is changed. Also, it will be convenient to watch files by mask and allow to specify several paths per rule (like, allow |
It also would be nice if there was a documented way to disable "w Enable watch" message in |
@ei-grad see docker/compose#11446 |
What this PR does / why we need it:
Introduce a new sub-section
develop
to collect developer workflow and how a Compose Implementation can hook into developer inner-loop to make developer experience better.update: this proposal has been adjusted to reflect experiment in Docker Compose with
x-develop
and thewatch
command