Skip to content
This repository has been archived by the owner on Jul 28, 2023. It is now read-only.

Support custom APPSODY_MOUNT separator #31

Open
gireeshpunathil opened this issue Sep 19, 2019 · 8 comments
Open

Support custom APPSODY_MOUNT separator #31

gireeshpunathil opened this issue Sep 19, 2019 · 8 comments
Labels
enhancement New feature or request priority/low Low impact issue

Comments

@gireeshpunathil
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Please refer to appsody/appsody#31

Describe the solution you'd like
Please refer to appsody/appsody#31

Describe alternatives you've considered
Please refer to appsody/appsody#31

Additional context
appsody/appsody#338 is raised in appsody, a similar change in controller in this repo would compliment it

@gireeshpunathil
Copy link
Contributor Author

cc @kewegner

@kylegc
Copy link
Member

kylegc commented Sep 20, 2019

@gireeshpunathil why would the controller need this? The controller does not run docker commands.

@kewegner
Copy link
Collaborator

The controller uses the APPSODY_MOUNTS environment variable in the case there are no APPSODY_WATCH_DIRS.

If we change the value of APPSODY_MOUNTS to have something other than ";" as a separator, the logic in the controller would have to change, because it splits on ";". See line 269 of main.go.

@kylegc
Copy link
Member

kylegc commented Sep 20, 2019

Ok, so to clarify, adopting the docker --mount syntax to allow colons does NOT need controller changes.

However if as a result we change the APPSODY_MOUNTS separator, then yes the controller needs to support this.

@kylegc kylegc changed the title Adopt a volume-mounting Docker syntax that allows colons in path names Support custom APPSODY_MOUNT separator Sep 20, 2019
@kewegner
Copy link
Collaborator

kewegner commented Dec 3, 2019

When this issue is done, we would also want to consider the separator for the APPSODY_WATCH_DIRs and the APPSODY_WATCH_IGNOREDIRS (PATHS). Since those are in a list using ';' as the separator.

@kewegner kewegner added the enhancement New feature or request label Dec 3, 2019
@neeraj-laad neeraj-laad added the priority/low Low impact issue label Dec 9, 2019
@neeraj-laad neeraj-laad added this to the Milestone-13 milestone Dec 9, 2019
@kewegner
Copy link
Collaborator

We should address #31 at the same time. Also the changes we made for single mount testing might be affected.

@kewegner
Copy link
Collaborator

I have started to code this up and will work with @rykal-nate prior to Friday.

@neeraj-laad
Copy link

We expect more changes in this area and would prefer to get a clear design agreed across various components - stacks, cli, controller etc. Other priorities have not allowed us to focus on this use case as much.

I've closed 2 PRs related to this, but once we have more clarity on how we want to enable this use case, we can re-open this PR again.

@neeraj-laad neeraj-laad removed this from the Milestone-14 milestone Jan 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request priority/low Low impact issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants