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 virtualenv provider to use curator via a Python virtualenv. #26

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

joshuar
Copy link

@joshuar joshuar commented Jan 15, 2015

This pull requests adds a new provider, "virtualenv" which allows for installing curator within a Python virtualenv. This is useful where installing curator and its dependencies conflict with distro-installed packages (i.e., conflicting versions).

The changes make use of the stankevich/python forge module and I've tried to ensure that it isn't a hard dependancy (i.e., not required if not using the virtualenv provider).

So if you don't use the virtualenv provider, the behaviour of the module has not changed.

@@ -121,7 +121,7 @@
# Copyright 2014 EvenUp.
#
define curator::job (
$path = '/usr/bin/curator',
$command = '/usr/bin/curator',
Copy link
Owner

Choose a reason for hiding this comment

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

This rename makes this a breaking change. I agree $path isn't as descriptive, is there a need to rename it at this time? I'd prefer to not have breaking changes unless there's a need to.

@jlambert121
Copy link
Owner

@joshuar Thanks for making changes to this. I think the broken tests now are because of some old Gemfile settings I can take care of. Would you mind looking at my comment in the job.pp (and squishing down to a single commit - see below)?

You can squish multiple commits into a single commit easily with git (rather than submitting a new PR if you'd like) by doing "git rebase -i HEAD~3" (the number is how many commits back from HEAD to include in your rebase). You can mark the later commits with a "f" for fixup (instead of pick) which will combine those commits into the previous commit. You then just have to do a "git push --force" (since you rewrote history) and it will update the PR.

Git rebase: http://git-scm.com/book/en/v2/Git-Branching-Rebasing

@joshuar
Copy link
Author

joshuar commented Jan 17, 2015

Hey @jlambert121,

Yeah, no need to change path. I'll change it back. Yep, sorry about closing/re-opening the issue, I stuffed my rebase up majorly and it was quicker to fork and copy the working copy changes across in a new pull request than fix the history.

Will rebase in this PR to squish everything to a single commit.

Thanks for the feedback and your patience!

@joshuar
Copy link
Author

joshuar commented Apr 1, 2015

Just wanted to check what the status of getting this PR merged is? The CI failures appear to be for old versions of Puppet? Happy to work on whatever is needed to get this merged.

@jlambert121
Copy link
Owner

I refactored this module for the newest version of curator and have been intending to go back and rework this PR and just haven't had a chance. If you'd like to take a look at it that would be awesome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants