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

Add wp core update-db to finalize hook #411

Merged
merged 1 commit into from
Dec 22, 2015

Conversation

mAAdhaTTah
Copy link
Contributor

Fixes #398.

Couple things: first, I think this is where this is supposed to go, but if I'm reading this right, it doesn't look like this file is actually run during the deployment process and works more like an example? And second, the reason I don't know the answer to that is I haven't fully tested it because I'm running an old version of Trellis that I need to upgrade to the latest variable configuration (you'll notice my fork is still named bedrock-ansible).

That said, I'm opening this up for comments, since this is my first Ansible task (which, as a side note, is a really neat benefit of the deployment hooks: it allows you to dip your toes into writing Ansible tasks without needing to understand everything at once).

Thoughts?

@mAAdhaTTah
Copy link
Contributor Author

Tested and working now.

when: wp_installed.rc == 0 and not (project.multisite.enabled | default(False) == True)

- name: Update WP network database
command: wp core update-db
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing --network here?

should we note that this could take a really long time for larger networks?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Musta gotten lost in a copy/paste.

Added a message. Let me know if you have a preference on the copy.

@mAAdhaTTah mAAdhaTTah force-pushed the wp-cli-update-db branch 2 times, most recently from 34fd1ee to faf1dab Compare November 14, 2015 18:14
command: wp core update-db
args:
chdir: "{{ deploy_helper.new_release_path }}"
when: wp_installed.rc == 0 and not (project.multisite.enabled | default(False) == True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These can all use when: wp_installed | success

Also shouldn't need last ==, so:

wp_installed | success and not (project.multisite.enabled | default(false))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do. I pulled wp_installed.rc == 0 from line 14; should I fix that too?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be great :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh and the parentheses aren't need either but it might be a subjective thing:

wp_installed | success and not project.multisite.enabled | default(false)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done and done.

@mAAdhaTTah mAAdhaTTah force-pushed the wp-cli-update-db branch 2 times, most recently from fcb6fef to b1c2ed0 Compare November 14, 2015 23:35
when: wp_installed | success and not project.multisite.enabled | default(false)

- name: Warn about updating network database.
debug: msg="Updating the network database could take a long time with a large number of sites."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

debug:
  msg:

:)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@swalkinshaw
Copy link
Member

This is looking good 👍 Probably will want to do some tests ourselves but it's pretty straightforward.

@swalkinshaw
Copy link
Member

@mAAdhaTTah this is good to go. Can you squash commits?

@mAAdhaTTah
Copy link
Contributor Author

@swalkinshaw done!

swalkinshaw added a commit that referenced this pull request Dec 22, 2015
Add `wp core update-db` to finalize hook
@swalkinshaw swalkinshaw merged commit b0ab880 into roots:master Dec 22, 2015
@swalkinshaw
Copy link
Member

Thanks!

@mAAdhaTTah mAAdhaTTah deleted the wp-cli-update-db branch December 22, 2015 22:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants