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

better Rails integration #134

Merged
merged 3 commits into from
Jul 11, 2013
Merged

Conversation

senny
Copy link
Contributor

@senny senny commented Mar 10, 2013

I finally had the time to work on a "smoother" Rails integration. This PR adds:

  • automatic loading of rake tasks
  • generator to create the migration file

Using queue_classic with Rails is now as simple as adding the gem and running rails g queue_classic:install.

I tested using a fresh rails 3.2.13.rc2 app.

@@ -1,4 +1,6 @@
task :environment
task :environment do
ActiveRecord::Base if defined?(Rails)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is necessary because I can't initialize the connection in the Railtie. (This would force Active Record to be loaded and a connection to be made when booting Rails). I'm not sure this is the best alternative though.

@ryandotsmith
Copy link
Contributor

@senny Have you considered the issues raised in #96 ?

@senny
Copy link
Contributor Author

senny commented Mar 10, 2013

@ryandotsmith no I did not consider the linked issue while implementing. It was more a proof of concept to see if you are interested in more automatic integration with rails. As mentioned in one of my comments, passing the connection is not optimal at the moment.

@ryandotsmith
Copy link
Contributor

@senny I see. I am excited to see the queue_classic setup become easier for rails developers.

👍

@senny
Copy link
Contributor Author

senny commented Mar 10, 2013

good. I'll think about a good way to pass the connection so that we don't need to load AR prematurely and we don't have the issues described in #96.

@baldrailers
Copy link

👍

@turadg
Copy link
Contributor

turadg commented Apr 8, 2013

I'm looking forward to this PR. It will help with a problem I've run into when my specs end up calling QC.enqueue: there's no DATABASE_URL and it raises an exception. I currently set DATABASE_URL using a .env with Foreman on my development box (and Heroku in production) but don't set it for the test environment. So the "automatic reuse of Active Records database connection" feature will help.

Further, I propose there be an option to put QC into a dummy mode so that when it gets asked to make a job, it merely confirms that it could but doesn't hit the database. I'm almost accomplishing this now through mocking, but it would be nice to be global. E.g. I have:

QC.should_receive(:enqueue) {Resource.scrape @resource.id}
# and then something happens that calls…
QC.enqueue "Resource.scrape", @resource.id

I would prefer to be able to leave the mock out of my specs and let QC infer it from the enqueue call. I can think of two "testing" modes that people might want.

  • Run immediately: Instead of hitting the db with a job that may not be complete in time for the test (if it even had a worker), just do the job in the current thread.
  • Dry run: Don't actually call Resource.scrape @resource.id, but confirm that Resource has a scrape method and the queue arguments fit its signature.

For the former, I figure something along these lines in spec helper:

# untested
QC.stub(:enqueue) do |method, *args|
  eval method + " #{args}"
end

@senny
Copy link
Contributor Author

senny commented Apr 9, 2013

@turadg I don't think we need to provide specific testing behavior. QC is tiny, focused and modular. It should be easiy to just adapt it to your testing needs. In my applications I always used custom MockQueue objects. Then you can decide what you want to do when a job is enqueued.

Skimming through the code I did not see a code to set the default_queue I think the default_queue should be assignable. This way you can inject the mock easily.

@turadg
Copy link
Contributor

turadg commented Apr 9, 2013

@senny any easier way to mock QC would be much appreciated. Can you post here or in the wiki an example of how you're mocking the queue now?

@senny
Copy link
Contributor Author

senny commented Apr 10, 2013

I always use wrapper code around QC::Queue instead of going through QC.enqueue directly. My wrapper code then makes it easy to switch out the Queue object it's working with to inject something when testing.

I then simply use different queues to verify different behavior. Sometimes to just store the jobs in an array to see if they were added, sometimes I wrap a real queue to do some introspection before. The Queue is the central piece and it's easy to create a specific Queue for testing needs.

I can't post the samples as they are properties of my clients but I think I'll write something up when I implement #default_queue= to illustrate it's purpose.

@turadg
Copy link
Contributor

turadg commented Apr 10, 2013

@senny I see. The default queue setter will definitely help. Thanks!

@senny
Copy link
Contributor Author

senny commented Apr 20, 2013

@turadg I just merged the patch to assign your own default_queue: #146 note that it wasn't released yet.

@senny
Copy link
Contributor Author

senny commented Jul 11, 2013

@ryandotsmith I removed the simplified connection handling from this PR to get it merged. I will take a look at #167 to see how we can integrate more easily with AR.

senny added a commit that referenced this pull request Jul 11, 2013
@senny senny merged commit 8c97ac1 into QueueClassic:master Jul 11, 2013
@senny senny deleted the rails_integration branch July 11, 2013 10:27
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.

None yet

4 participants