Skip to content
This repository has been archived by the owner on Oct 10, 2021. It is now read-only.

Update Gemini config for https://github.com/Islandora/Crayfish/pull/109 #36

Merged
merged 6 commits into from
Dec 15, 2020

Conversation

ruebot
Copy link
Contributor

@ruebot ruebot commented Dec 2, 2020

GitHub Issue: Islandora/Crayfish#109

What does this Pull Request do?

Update Gemini config for Islandora/Crayfish#109

  • Remove crayfish_gemini_fedora_base_url
  • Add crayfish_gemini_fedora_domain
  • Add crayfish_gemini_drupal_domain

Hit this problem this afternoon building a new machine:

[2020-12-02 18:29:58] app.CRITICAL: Pimple\Exception\UnknownIdentifierException: Identifier "crayfish.drupal_domain" is not defined. (uncaught exception) at /var/www/html/Crayfish/Gemini/vendor/pimple/pimple/src/Pimple/Container.php line 101 {"exception":"[object] (Pimple\\Exception\\UnknownIdentifierException(code: 0): Identifier \"crayfish.drupal_domain\" is not defined. at /var/www/html/Crayfish/Gemini/vendor/pimple/pimple/src/Pimple/Container.php:101)"} []
[2020-12-02 18:29:58] app.DEBUG: < 500 [] []

Interested parties

@seth-shaw-unlv @dannylamb

- Remove crayfish_gemini_fedora_base_url
- Add crayfish_gemini_fedora_domain
- Add crayfish_gemini_drupal_domain
Copy link
Contributor

@seth-shaw-unlv seth-shaw-unlv left a comment

Choose a reason for hiding this comment

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

Smoke tested with no issues. 👍

@seth-shaw-unlv
Copy link
Contributor

Travis had an issue downloading a part. I've restarted the build. Hopefully it will work this time.

@seth-shaw-unlv
Copy link
Contributor

@dannylamb, Travis is failing here because ansible-galaxy is still attempting to install the "master" branch (which no longer exists). I don't see a way to tell galaxy to change this behavior. The tests do have their own requirements file, but it isn't structured like the playbook where we can specify a version/branch. 😕

Any ideas on fixing this?

@dannylamb
Copy link
Contributor

@ruebot Do you think you could put this chunk in tests/requirements.yml for the keymaster role. I think you just need to explicitly declare that you're using the main branch and it'll all sort out.

@ruebot
Copy link
Contributor Author

ruebot commented Dec 4, 2020

@dannylamb sure thing!

@seth-shaw-unlv
Copy link
Contributor

Travis is taking it's time getting around to testing this. I'll keep my eye on it.

@seth-shaw-unlv
Copy link
Contributor

Travis failed again. Apparently we don't have git on the test image?

[WARNING]: - Islandora-Devops.keymaster was NOT installed successfully: could not find/use git, it is required to continue with installing https://github.com/Islandora-Devops/ansible-role-keymaster

@dannylamb
Copy link
Contributor

🤦‍♂️

@ruebot
Copy link
Contributor Author

ruebot commented Dec 8, 2020

@dannylamb @seth-shaw-unlv I was looking at the TravisCI config files to figure out the testing infrastructure on the Islandora-Devops roles. Looks like a lot, if not all of them are on 16.04. Do they need to need to be updated to at least 18.04 to match with the default configuration in the vagrant file? Though, I'm not sure if that would address the more immediate concern of git being installed.

@seth-shaw-unlv
Copy link
Contributor

@ruebot, worth a shot!

@ruebot
Copy link
Contributor Author

ruebot commented Dec 9, 2020

I just realized I was pushing to the canonical repo, instead of our fork. Sorry about that y'all. Really embarrassing on my part, and glad I didn't push to main 🤦‍♂️ 🙏

@dannylamb
Copy link
Contributor

main is protected, but anybody can push a topic branch to our canonical repo for testing. No big deal!

@seth-shaw-unlv
Copy link
Contributor

@dannylamb & @ruebot, looking at the failure some more... @ruebot updated the tests to use 7.4, but it appears the Crayfish composer lockfile is imposing constraints that prevent it from running on 7.4 due to dependency issues, especially with the move to Composer 2.

So, we need to get a Crayfish PR that brings things up to 7.4... which now makes it part of the D9 push.

@ruebot
Copy link
Contributor Author

ruebot commented Dec 10, 2020

@seth-shaw-unlv that's my end goal for Islandora/Crayfish#110 now. Just waiting on the Chullo PR to sort out, and then I'll jump on this one.

- crayfish_version_tag variable is set to dev for the tests (default is
still 1.0.0)
- add crayfish_fedora_base_url as a varaible since it is called multiple
times, but never defined
- ansible lint cleanup
- don't install psql on mysql playbook and vice versa
@seth-shaw-unlv
Copy link
Contributor

As long as Travis comes back green, this will be good.

@ruebot
Copy link
Contributor Author

ruebot commented Dec 14, 2020

I think we're getting the Idempotence test: fail because of these two spots on the second run:

  1. https://travis-ci.org/github/Islandora-Devops/ansible-role-crayfish/jobs/749705936#L1374

    • this has a force, so if I understand ansible correctly, that will always fail an Idempotence test because it is a force.
  2. https://travis-ci.org/github/Islandora-Devops/ansible-role-crayfish/jobs/749705936#L1438-L1441

    • Similar to the above, I think the way the "Template Symfony config files" task is written will also always fail an Idempotence test because anytime one of the items in the loop is a file, it's going to modify it. So, it's going to always be changed.

@seth-shaw-unlv @dannylamb lt me know how you want to handle this. I'm not really around until January 7th. But, happy to push this across the finish line if y'all want further action.

@seth-shaw-unlv
Copy link
Contributor

I thought you were already gone! I was just looking at this. I think we will want to get these two bits resolved, but since we can push to your branch we should be able to work it though. Go enjoy your vacation! 😀

@seth-shaw-unlv
Copy link
Contributor

Huh, we've been ignoring idempotency since 2782ab0 back in February when the force flag was added for 're-runability'. So, I guess if everyone else is fine with it, we will run with it. We can open a new issue if folks decide we need to revisit this.

You've done well, @ruebot, sir.

@seth-shaw-unlv seth-shaw-unlv merged commit 1c902e2 into main Dec 15, 2020
@ruebot
Copy link
Contributor Author

ruebot commented Dec 15, 2020

Thanks @seth-shaw-unlv 🙏

@ruebot ruebot deleted the Crayfish/pull/109 branch December 15, 2020 01:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants