use php console migrations:migrate to install gemini table #9
Conversation
|
Oh, Travis. But I don't want to set "crayfish_version_tag: master" in defaults/main.yml just to make you happy. |
|
@DiegoPino, so Travis isn't finding the postgres PDO driver for testing. Any idea on how to appease Travis? @dannylamb, I don't suppose you have any ideas on how to fix this? |
|
@seth-shaw-unlv any chance you are not deploying PHP driver via ansible? dbal is just an abstraction of that one. So in Ubuntu it would be |
|
@seth-shaw-unlv probably we need to set from https://github.com/geerlingguy/ansible-role-php some |
|
Probably something like that. The playbook sets these, which is why they work in my testing. I guess since we previously didn't run any code in this ansible role it wasn't necessary for idempotency. |
|
So this leads me to the question, how do we set it for testing? Travis is running the geerlingguy roles before it starts on ours, which means we can't use the usual strategy of importing OS-specific variables from the vars dir. Instead, I guess, we may need to set it in defaults/main.yml... unless we can do it in the travis configuration(?). |
Trying to appease Travis CI
|
OK @seth-shaw-unlv nice, this is much better. Now it is failing because migration file does not allow for Postgres in my code. I will make a pull tomorrow fixing that on gemini =)
|
|
@DiegoPino I already have a branch for it (Islandora-CLAW/Crayfish/tree/issue-828). I was waiting for this PR to clear first, but it looks like I need to reverse the order of the PRs. |
|
Travis won't clear until Islandora/Crayfish/pull/47 is merged. |
|
Can someone (possibly @dannylamb?) restart a Travis build? I don't seem to have the ability to restart it. It should work now that Islandora/Crayfish#47 is merged. |
|
@seth-shaw-unlv I restarted the build for you, but you should have those permissions. I"ll dig through the admin settings here and see why you can't do it. In theory, it should all flow from Github to Travis, so I'm a bit puzzled as to why you can't. |
|
@seth-shaw-unlv So I just went through and touched up perms for all the repos in the Islandora-Devops organization. Let me know if you still can't restart travis builds. |
|
Okay, so it is now failing the geerlingguy idempotence test, and I'm not sure what to do about it. In part because I'm not sure what part is breaking it (the build log doesn't provide any help on that point). Any hints, @dannylamb or @whikloj, about what can be adjusted to fix this? |
|
Hi @seth-shaw-unlv. What the idempotence test does is to re-run the playbook immediately after its been run to make sure that no tasks show a 'changed' status, to ensure idempotence. You can see the tasks that shows changed here: https://travis-ci.org/Islandora-Devops/ansible-role-crayfish/jobs/386873469#L1682 This is an issue because the Gemini migrate db task uses the I would attach a when condition to this task so it knows when to run the migration. Not sure the correct logic here, but something like this would fix the idempotence issue: - name: Gemini composer install
composer:
command: install
working_dir: "{{ crayfish_install_dir }}/Gemini"
register: gemini_install
- name: Gemini migrate db
shell: php bin/console migrations:migrate
args:
chdir: "{{ crayfish_install_dir }}/Gemini"
when: gemini_install.changedSo that would only run the migration if the composer package has changed. |
defaults/main.yml
Outdated
| @@ -1,4 +1,4 @@ | |||
| crayfish_version_tag: 0.0.8 | |||
| crayfish_version_tag: master | |||
There was a problem hiding this comment.
You can make this 0.0.9 as @dannylamb cut a release with the postgresql migrations in it.
https://github.com/Islandora-CLAW/Crayfish/tree/0.0.9
|
So what I need to do is see if the db has the current schema before running the migration, without using the shell... I'll see what I can cook up tomorrow. |
|
@seth-shaw-unlv you can use Doctrine for that too. Schema creates a supporting table that holds which version of the schema was lastly applied. Let me know if you need help, if, at all, I can also share some example files with you. |
|
@DiegoPino the problem with using Doctrine for this, as far as I understand it, is I would still need to put a call out via the shell to use the console. I need to find a solution using an ansible module to pass the idempotence test.. |
|
@seth-shaw-unlv I think the logic I have there works, because if the composer package hasn't changed, there is no way that the schema has changed. If the composer package has changed, it doesn't mean the schema has for sure changed, but I think its safe to run the shell command. |
|
@jonathangreen Oh! I get it now. Let me make that modification and spin it up locally; then I'll update the PR. |
|
Actually, the logic doesn't work in this case, because gemini composer install task returns "ok" on the first pass, so the migrate step gets skipped by default and doesn't run at all. I'm testing out overriding the shell's changed result. |
|
🤞 |
|
YAY! Can someone review and merge, please? @whikloj |
|
This looks good to me and if Travis is passing that's good. Fortunately/unfortunately I am in an airport and unlikely to build this in the next couple days. Sorry |
|
@Islandora-Devops/committers anyone able to review/merge this, please? |
| args: | ||
| chdir: "{{ crayfish_install_dir }}/Gemini" | ||
| register: gemini_migrate | ||
| changed_when: '"No migrations to execute." not in gemini_migrate.stdout' |
There was a problem hiding this comment.
Without testing this seems wrong. Single quotes won't evaluate the not in inside the string
So could be maybe
changed_when: "gemini_migrate.stdou is not search('No migrations to execute')" ? or any other option, but single quotes around everything seem to me that would evaluate to a string instead.
There was a problem hiding this comment.
It does indeed work as written. At least, it worked for my own tests and the Travis builds.
Another example of single-quotes encapsulating a condition for changed_when can be found in the "Convergence and command/shell" section (changed_when subsection) of the blog post "Using Ansible's command and shell modules properly".
There was a problem hiding this comment.
Are your sure it is not evaluating to false always? I believe you and makes sense for running shell commands, but for internal evaluations like {{ }} or not in, seems ambiguous. Will add a debug there and test.
There was a problem hiding this comment.
I just scoured the ansible documentation and found nothing that suggested single-quotes will always evaluate to a string.
It does look like I could have omitted the single-quote altogether; but I hesitate to change it on speculation when Travis is already content.
There was a problem hiding this comment.
According to the Travis build the first pass did execute the task while the second did not which implies to me the logic works (and isn't always evaluating false).
There was a problem hiding this comment.
Ok, i trust you, will approve. If ansible handles double quotes and single quotes as the same, by expanding statement and even variable evaluation, then i feel something is wrong with Ansible. This can lead to very problematic issues. And i may not be the only one https://zncb.me/blog/2016/03/11/sneaky-ansible-pitfalls/
GitHub Issue: Islandora/documentation/issues/722
See also Islandora/Crayfish/pull/46
What does this Pull Request do?
Updates the ansible role to run the migrations:migrate command to generate Gemini's table.
How should this be tested?
Note: if you want to test this with the new content-model-overhaul, you will need to merge Crayfish master into @dannylamb's content-model-overhaul branch after running his install gist.
Additional Notes:
N/A
Interested parties
@DiegoPino, @DigitLib, @ajs6f, @Islandora-CLAW/committers