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

Resque scheduler #93

Merged
merged 4 commits into from
Feb 15, 2017
Merged

Resque scheduler #93

merged 4 commits into from
Feb 15, 2017

Conversation

crigor
Copy link
Contributor

@crigor crigor commented Dec 8, 2016

Uses Joe Heth's code from #75

@crigor crigor removed the WIP label Feb 9, 2017
@dbeckstead dbeckstead merged commit 859b99a into next-release Feb 15, 2017
@@ -0,0 +1 @@
include_recipe 'resque'
Copy link
Contributor

Choose a reason for hiding this comment

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

it should be include_recipe 'resque_scheduler'

@dvalfre
Copy link
Contributor

dvalfre commented Feb 20, 2017

The main 'resque-scheduler' recipe has the following code:

  execute 'install resque gem' do
    command 'gem install resque redis redis-namespace yajl-ruby -r'
    not_if { 'gem list | grep resque' }
end

Which comes from the v4 recipe. According to engineyard/ey-cloud-recipes#150 that codes is faulty and evaluates to '1', which is why the gems get never installed.
Funnily enough, the 'resque-scheduler' works (in v4, and guess will do in v5) since the necessary Gems are included on the app's Gemfile.

Shall the aforemenationed code be dropped from the main 'resque-scheduler' recipe?

@radamanthus
Copy link
Contributor

@dvalfre I agree, the gem dependencies should be in the Gemfile and the recipe should not bother with installing the gems.

@@ -0,0 +1 @@
default['redis']['utility_name'] = 'resque'
Copy link
Contributor

Choose a reason for hiding this comment

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

For a clustered env, this line should be replaced with at least:

default['redis'].tap do |redis|

  # Collect the redis instances in this array
  redis_instances = []

  # Run Redis on a named util instance
  # This is the default
  redis['utility_name'] = 'resque'
  redis_instances << redis['utility_name']
  redis['is_redis_instance'] = (
    node['dna']['instance_role'] == 'util' &&
    redis_instances.include?(node['dna']['name'])
  )
end

and for a solo env with:

default['redis'].tap do |redis|

  # Collect the redis instances in this array
  redis_instances = []

 # Run redis on a solo instance
  # Not recommended for production environments
  #redis['is_redis_instance'] = (node['dna']['instance_role'] == 'solo') 
end

@@ -0,0 +1,3 @@
# Custom Redis

This custom-redis recipe is used in the resque example to install redis on the utility instance named "resque" instead of the default "redis".
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we include this small Redis wrapper at all? Leaning towards adding a section on the Resque-Scheduler wrapper's Readme telling the customer to enable the full Redis recipe (as we do with Sidekiq).

Copy link
Contributor

Choose a reason for hiding this comment

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

I opened PR #150 to remove custom-redis from resque_scheduler

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.

5 participants