Updated devstack nfs make targets #483
Conversation
UsamaSadiq force-pushed the usamasadiq/bom-1194 branch from 203e0cc to beb8a40 Compare 
docker-compose-themes-nfs.yml Outdated
| - edx-nfs:/edx/app/edx-themes:cached | ||
| | ||
| volumes: | ||
| edx-nfs: |
There was a problem hiding this comment.
I'm not sure it's safe to use the same volume name as in docker-compose-host-nfs.yml with a different directory; maybe use something like themes-nfs instead?
There was a problem hiding this comment.
okay, I'll update edx-nfs to themes-nfs for safety.
Makefile Outdated
| | ||
| dev.nfs.up: | check-memory ## Bring up all services with host volumes | ||
| docker-compose -f docker-compose.yml -f docker-compose-host-nfs.yml up -d | ||
| docker-compose -f docker-compose.yml -f docker-compose-host-nfs.yml -f docker-compose-themes-nfs.yml up -d |
There was a problem hiding this comment.
Could you create a new variable like DOCKER_COMPOSE_FILES_NFS to capture these (similar to the existing DOCKER_COMPOSE_FILES) so we don't need to keep copying this list in different targets?
There was a problem hiding this comment.
Sure, I was also thinking the same. I'll create another variable for the nfs arguments.
Makefile Outdated
| | ||
| dev.nfs.provision: | check-memory dev.clone dev.provision.nfs.run stop ## Provision dev environment with all services stopped | ||
| | ||
| dev.provision.nfs.run: ## Provision all services with local mounted directories |
There was a problem hiding this comment.
The original target this was copied from has since been renamed from dev.provision.run to dev.provision.services, this should be updated to match, maybe to dev.nfs.provision.services. There should also be a dev.nfs.provision.services.% equivalent for dev.provision.services.% which limits which services are provisioned.
There was a problem hiding this comment.
Renamed dev.provision.nfs.run to dev.nfs.provision.services.
Added dev.nfs.provision.services.*.
Makefile Outdated
| | ||
| dev.nfs.up.all: | dev.nfs.up dev.nfs.up.watchers ## Bring up all services with host volumes, including watchers | ||
| | ||
| dev.nfs.provision: | check-memory dev.clone dev.provision.nfs.run stop ## Provision dev environment with all services stopped |
There was a problem hiding this comment.
Need to match the rename of the target below in the dependencies here.
There was a problem hiding this comment.
renamed to NFS_COMPOSE_FILES.
There was a problem hiding this comment.
What I meant was that this line includes "dev.provision.nfs.run", but that target has been renamed to "dev.nfs.provision.services"; it needs to be renamed here also.
Makefile Outdated
| | ||
| STANDARD_COMPOSE_FILES=-f docker-compose.yml -f docker-compose-host.yml -f docker-compose-themes.yml | ||
| | ||
| NFS_COMPOSED_FILES=-f docker-compose.yml -f docker-compose-host-nfs.yml -f docker-compose-themes-nfs.yml |
There was a problem hiding this comment.
Minor suggestion: could this be NFS_COMPOSE_FILES instead for consistency with the other variable?
There was a problem hiding this comment.
Renamed to NFS_COMPOSE_FILES.
Makefile Outdated
| dev.nfs.provision.services: ## Provision all services with local mounted directories | ||
| DOCKER_COMPOSE_FILES=$(NFS_COMPOSED_FILES) ./provision.sh | ||
| | ||
| dev.nfs.provision.services.*: ## Provision specified services with local mounted directories, separated by plus signs |
There was a problem hiding this comment.
I think this needs to be .%, not .*
There was a problem hiding this comment.
renamed to dev.nfs.provision.services.%.
UsamaSadiq force-pushed the usamasadiq/bom-1194 branch from 2f87659 to 11c555e Compare UsamaSadiq force-pushed the usamasadiq/bom-1194 branch from 11c555e to f3b694e Compare 
Issue: BOM-1194
Description
Changed Commands