Skip to content
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

Craft Storage Folder as a Variable #40

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

dwhoban
Copy link

@dwhoban dwhoban commented Sep 16, 2018

Currently, the storage folder is fixed to ${LOCAL_ROOT_PATH}"${GLOBAL_CRAFT_PATH}storage/" & ${REMOTE_ROOT_PATH}"${GLOBAL_CRAFT_PATH}storage/" which has some limitations if using atomic deployments where the storage folder is symlinked into the release folder.

This change uses the same location as before but moving this variable to the .env.sh file allowing for it to be configured to an absolute path and synced without error.

@khalwat
Copy link
Contributor

khalwat commented Sep 18, 2018

So what we did for this for atomic deployments was we just symlink'd the storage directory; would this be a feasible solution for you?

@dwhoban
Copy link
Author

dwhoban commented Sep 24, 2018

That's kinda the reason for the change. I had been symlinking uploaded assets and storage to the correct positions. I am using atomic deployments on both production and staging.

When using pull-db.sh and pull-assets.sh in my staging server the commands were failing at the mkdir command unless I pointed at the absolute location of the folders and not the symlink.

Might be a better way to handle this issue, but this was the simplest fix I could think of

@khalwat
Copy link
Contributor

khalwat commented Sep 25, 2018

Gotcha, makes sense!

@dwhoban
Copy link
Author

dwhoban commented Dec 29, 2018

Any chance this will get merged?

@khalwat
Copy link
Contributor

khalwat commented Jan 5, 2019

I haven't had time to evaluate the implications of it yet; and I'd also like to see if perhaps there isn't a way we can make this work without adding yet another variable to the config

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants