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

Use external roles #43

Merged
merged 3 commits into from Jan 17, 2018

Conversation

Projects
None yet
4 participants
@whikloj
Copy link
Member

commented Dec 20, 2017

Resolves Islandora-CLAW/CLAW#764

This moves all the internal roles out to the external ones.

This has introduced a bug documented here where you (or at least I) can't download all the external roles without a failure from galaxy (or Github...which is my guess).

How To Test

  1. Clone this branch to a new directory.
  2. Run vagrant up. It will fail (or it always does for me on the 21st role)
  3. Run vagrant provision

You should have a working CLAW instance.

@dannylamb

This comment has been minimized.

Copy link
Contributor

commented Dec 21, 2017

vagrant up

Let's see what happens.

@dannylamb

This comment has been minimized.

Copy link
Contributor

commented Dec 21, 2017

😕 Hrm... I just made it past ansible-galaxy unscathed and didn't have to force provisioning. I'll let you know when it's totally done if it's fine.

@whikloj

This comment has been minimized.

Copy link
Member Author

commented Dec 21, 2017

Did you delete your roles/external directory?

If not then you didn't have to re-download all of the geerlingguy roles and you only downloaded our 6 new roles. Hence not hitting the 21 limit.

@dannylamb

This comment has been minimized.

Copy link
Contributor

commented Dec 21, 2017

Ah, ok. Let me try that.

@dannylamb

This comment has been minimized.

Copy link
Contributor

commented Dec 21, 2017

🤷‍♂ I dunno, just tried it twice, clobbering roles/external and it worked fine both times.

@whikloj

This comment has been minimized.

Copy link
Member Author

commented Dec 21, 2017

What dark pact did you make?!?!!?!

@dannylamb

This comment has been minimized.

Copy link
Contributor

commented Dec 22, 2017

Me and Beezlebot go way back.

But it's probably just my ultra-slow internet narrowly avoiding whatever rate limit they've set.

@Natkeeran

This comment has been minimized.

Copy link
Contributor

commented Jan 5, 2018

Please ignore my previous comment. vagrant up works with this PR. No errors. Basic CRUD (create a resource, sync to Fedora, Triplestore) works as expected.

@dannylamb

This comment has been minimized.

Copy link
Contributor

commented Jan 8, 2018

Are we cool to move this in, then?

@Natkeeran

This comment has been minimized.

Copy link
Contributor

commented Jan 9, 2018

@dannylamb Yes, Works as expected for me. In addition to this one, if 40 and 44 can be merged, then claw-vagrant can be decommissioned.

@dannylamb

This comment has been minimized.

Copy link
Contributor

commented Jan 10, 2018

@whikloj @Natkeeran @seth-shaw-unlv Ok, before this goes in then, let's take a look at what we've got going and see if there's other stuff we can sneak in first before I unleash some git-fu on all the repositories for the split out roles.

@whikloj

This comment has been minimized.

Copy link
Member Author

commented Jan 10, 2018

Wait?! What git-fu are your doing?

The plan (as I understood it) was to NOT do anything to claw-playbook until we got this merged and the external roles in use. Then I was going to start on PRs to clean up all the variable overlap that was happening in the various roles.

@whikloj

This comment has been minimized.

Copy link
Member Author

commented Jan 10, 2018

So any PRs for the individual roles could be recreated on their specific repository (ie #44), but then it would be easier to test as we would actually be using the external roles here.

@Natkeeran

This comment has been minimized.

Copy link
Contributor

commented Jan 10, 2018

@whikloj Yes, that approach seems good to me.

@dannylamb

This comment has been minimized.

Copy link
Contributor

commented Jan 10, 2018

I was totally going to just force push whatever changes have already rolled through to the individual repos and merge the PR after testing again. Just sense it's stuff we've already reviewed. Subsequent work would still go through the normal workflow for the applicable role(s).

@whikloj

This comment has been minimized.

Copy link
Member Author

commented Jan 10, 2018

@dannylamb But you shouldn't need to force push anything. I rebuilt all the external role repositories as exact copies of what is currently in claw-playbook (I had to add the meta/main.yml to some). But otherwise this PR should merge without any issues and once that happens we are moved to using external roles.

@jonathangreen

This comment has been minimized.

Copy link
Contributor

commented Jan 11, 2018

Just going to throw this here to tie the two discussions together. I can't get the roles to come in cleanly, same error as Jared had. I made a PR on his PR here: whikloj#1 that pulls them from git instead of galaxy and that works okay for me. I dunno if thats an okay solution for everyone.

@jonathangreen

This comment has been minimized.

Copy link
Contributor

commented Jan 11, 2018

So this comes up fine for me after two things:

  1. switching to git for some roles: whikloj#1
  2. commenting out FITS: Islandora-CLAW/CLAW#747
    • PR for this here: #46

I guess I can't merge this anymore because I'm implicated in it now, but I'm satisfied and would merge this after the two PR above are merged.

@whikloj

This comment has been minimized.

Copy link
Member Author

commented Jan 12, 2018

I'm going to have to rebase this PR after the FITS removal, so do we have any issues with switching the galaxy roles to Github repos?

If we don't I can merge @jonathangreen's PR.

Personally I'm a little concerned about the fact that Galaxy seems to be our failure point.

@whikloj whikloj force-pushed the whikloj:issue-764 branch from b6df537 to b57810e Jan 12, 2018

@whikloj

This comment has been minimized.

Copy link
Member Author

commented Jan 12, 2018

@dannylamb I just went ahead and merged @jonathangreen's PR. It solves the problem for him and me (maybe a OSX issue) and now we are good to go.

@Natkeeran

This comment has been minimized.

Copy link
Contributor

commented Jan 17, 2018

@dannylamb Can we merge this one!

@dannylamb dannylamb merged commit 8ace255 into Islandora-Devops:master Jan 17, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.