valentinocossar / trellis-database-uploads-migration

Ansible playbook for Trellis that manages database and uploads migration.
MIT License
94 stars 13 forks source link

Alternative hosts configuration #17

Closed handpressed closed 6 years ago

handpressed commented 6 years ago

Added an alternative hosts configuration to README.md for users encountering errors with database.sh pull and push.

Plus some minor typos, and an issue with remote db backups not being removed (deleting 'delegate_to: development_host' from database-backup.yml line 56 fixed it - needs testing).

valentinocossar commented 6 years ago

Thank you very much @handpressed. I'll test and review the pull request as soon as possible. I would like to provide only one way to configure the hosts in the README to prevent confusion and errors for new users. I'll take some time to test it well.

handpressed commented 6 years ago

I agree on the need to provide only one way to configure the hosts. Both ways are working for me now. I'll do some more testing too, see if I can find where the original wordpress_sites variable error was happening when using the original hosts config.

valentinocossar commented 6 years ago

@handpressed thank you so much for your help, I would like to see if declaring the ansible_host for every host group is something possible and if it solves the problem.

handpressed commented 6 years ago

All three of these host configs work for me (including the original which didn't at first and led to this pull request):

Original:

production_host ansible_host=your_server_hostname

[production]
production_host

[web]
production_host

First fix:

[production]
production_host ansible_host=your_server_hostname
your_server_hostname

[web]
your_server_hostname

Second fix:

[production]
production_host ansible_host=your_server_hostname
production_host

[web]
production_host

First and second fixes are essentially the same - I'm sticking with the second and have updated the fork's README to reflect this. It would be nice to get some more users to test it though.

Adding the ansible_host to both groups works for both the first and second fixes but doesn't seem to be necessary.

handpressed commented 6 years ago

Simplified it further by removing the second production_host variable from the production group:

[production]
production_host ansible_host=your_server_hostname

[web]
production_host

Works fine. Have updated fork's README.

valentinocossar commented 6 years ago

Thank you very much @handpressed, this update makes sense to me. I'll look into it as soon as possible, I prefer a lot of tests to prevent errors in production. Right now I have new projects where I can try this new fix and (maybe) I've found who can test it also on a new Linux distro. I'll update this pull request as soon as I've further details.

valentinocossar commented 6 years ago

Hi @handpressed, latest commits that you have done are being automatically added to this pull request and have errors in it. Latest release url should not be changed and the CHANGELOG release is wrong because I've not merged the pull request. Also to update the CHANGELOG you have to replace HEAD with the release version and date. But I'll update the CHANGELOG as soon as we find the best solution to close and merge this pull request.

Anyway, maybe an approach like this would be the better solution to keep the same Trellis logic:

[production]
production_host ansible_host=your_server_hostname

[web]
production_host ansible_host=your_server_hostname

What do you think? I've not tested it yet. But even if it is redundant, it keeps the same logic that Trellis uses in its hosts' configuration.

handpressed commented 6 years ago

Hi @valentinocossar - Apologies. It's easy to forget that further commits are automatically added to pull requests. I've rolled back to the last minor typo correction and deleted the release.

I tested your new suggestion last night and I've just tried it again and it works fine for me. If it's consistent with Trellis logic then it's probably the best way to go. I'll leave the fork alone until we find the best solution.

valentinocossar commented 6 years ago

Nevermind, thank you for all your help, I'll try my best to merge this as soon as possible.

valentinocossar commented 6 years ago

Hi @handpressed, I've just pushed an update to the hosts configuration to keep the same logic that Trellis uses in its hosts configuration (as mentioned above). What do you think? Could you test this new update? I've tested it on some projects and it works well for me. If there's no particulars concerns I would like to proceed with the merge.

Thank you!

handpressed commented 6 years ago

Hi @valentinocossar - The update works fine for me too.

valentinocossar commented 6 years ago

Hi @handpressed, thank you! I've updated the CHANGELOG and now I merge your pull request.