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 enqueue to Qs and add to Queue #38

Merged
merged 1 commit into from
Jan 2, 2015
Merged

Add enqueue to Qs and add to Queue #38

merged 1 commit into from
Jan 2, 2015

Conversation

jcredding
Copy link
Member

This adds an enqueue method to Qs and add to Queue. These
allow adding jobs to queues in redis to be processed by daemons.
The enqueue method on Qs takes a queue name, a job name and
job params. The add method on Queue only takes a job name and
job params. This allows multiple convenient methods to add jobs
to a queue to be processed.

@kellyredding - Ready for review.

self.redis_lpush(queue_redis_key, job_name, params)
end

def self.add_job(*args)
Copy link
Member

Choose a reason for hiding this comment

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

@jcredding do we need this one? There is no mention of it in the commit msg (I'd prefer to not have it as I want to centralize our verbs around "enqueue" for jobs and "publish" for events).

@kellyredding
Copy link
Member

@jcredding I made a few comments around simplifying and reducing the API footprint? Weigh in with and comments or updates and I'll look again.

@jcredding
Copy link
Member Author

@kellyredding - Overall, I'm fine with simplifying the API. I'm cool with requiring passing a Queue object to the method, that greatly simplifies the implementation but is less like resque's interface which is what I was mimicking. I'll make the changes and we can discuss further.

@jcredding jcredding force-pushed the jcr-enqueue branch 5 times, most recently from 99f25a0 to 5498673 Compare December 30, 2014 20:08
@jcredding
Copy link
Member Author

@kellyredding - I've updated this with the simpler interface and changed the tests to use the HellaRedis::ConnectionSpy.

job = Qs::Job.new(job_name, params || {})
encoded_payload = Qs::Payload.encode(job.to_payload)
self.redis.with{ |c| c.lpush(queue.redis_key, encoded_payload) }
end
Copy link
Member

Choose a reason for hiding this comment

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

@jcredding let's have this return the job object that is being enqueued. Tap or whatever. Test for it.

@kellyredding
Copy link
Member

@jcredding update the enqueue method to return the job; update the enqueue tests and the add tests to make sure a job is returned. Otherwise I'm good here 👎 .

This adds an `enqueue` method to `Qs` and `add` to `Queue`. These
allow adding jobs to queues in redis to be processed by daemons.
The `enqueue` method on `Qs` takes a queue name, a job name and
job params. The `add` method on `Queue` only takes a job name and
job params. This allows multiple convenient methods to add jobs
to a queue to be processed.
jcredding added a commit that referenced this pull request Jan 2, 2015
Add `enqueue` to `Qs` and `add` to `Queue`
@jcredding jcredding merged commit 960c4b2 into master Jan 2, 2015
@jcredding jcredding deleted the jcr-enqueue branch January 2, 2015 15:42
jcredding added a commit that referenced this pull request Apr 24, 2015
* gem created with ggem (7b911ec)
* setting up the gemspec (52da24d)
* Add a `qs` bin and `CLI` class for it's logic (#4)
* minor updates to the gem meta (#5)
* Implement `Config` for evaluated config files (#6)
* Add a `Process` class for managing Qs daemon processes (#7)
* Make `Qs::Daemon` a mixin, not a class (#9)
* Add daemon configuration, provides options and validation (#10)
* Update daemon config and `Process` interaction (#11)
* Update `Daemon` state and `Process` signal handling (#12)
* Add `Daemon` work loop that uses `DatWorkerPool` (#13)
* Add `Worker` class; parses job, finds handler and runs it (#14)
* Add `ErrorHandler`, calls a list of procs when a job fails (#15)
* Change `Daemon` state handling, rename to "signal" (#16)
* Use dat-worker-pool's spy in the daemon tests (#17)
* Simply CLI `Config` class (#18)
* Use dependency-injection with `CLI` and ruby's `Kernel` (#19)
* Update to latest gem conventions (#20)
* Update `CLI`, add `ConfigFile` (#21)
* Add `ProcessSignal` and `PIDFile` (#22)
* Change `Daemon` module organization (5cbc537)
* Remove `Config` tests (66c621c)
* Add `Queue` and `Route` (#23)
* Add `Logger` and `NullLogger` (#25)
* Update and simplify `Process` (#24)
* Add Redis configuration and connection (#27)
* Add `DaemonData` (#26)
* Add `JobHandler` mixin (#28)
* Add runners for live and test environments (#29)
* Add `Job`, handles converting to/from payloads (#30)
* Update `ErrorHandler`, match Sanford error handler (#31)
* Implement `Route#run` (#33)
* Add `PayloadHandler`, processes a serialized payload (#32)
* Add `Daemon::Configuration` (#34)
* Add DSL methods for configuring a `Daemon` (#35)
* Update `Daemon` with start, stop and halt (#36)
* Update to use the latest HellaRedis (#39)
* Add `enqueue` to `Qs` and `add` to `Queue` (#38)
* Add redis connection and worker pool logic to `Daemon` (#37)
* Remove `TmpDaemon`, use `Daemon` instead (#40)
* Properly create/close IO pipes on start/shutdown (#41)
* Add system tests for `Daemon` (#42)
* Add benchmarking script (#43)
* Add `TestHelpers` for mixin for building a runner/handler (#44)
* Add `TestClient` and test mode, track enqueued jobs (#45)
* Allow configuring a serializer/deserializer (#47)
* Add job timeouts (#46)
* Change restart cmd to check if `ENV['PWD']` is set (#49)
* require daemon (ec77ffb)
* Expand `ErrorHandler` to take more information (#50)
* Add `push` and `prepend` to `Client` (#52)
* Expand redis item to track the state of processing it (#51)
* Update `Daemon` to build a client (#53)
* Job's payload, convert name to a string (e1ada59)
* Update to the latest `DatWorkerPool` (#54)
* Add `Qs.push` for adding payloads to queues (#56)
* Serialize when enqueuing and building test runner (#55)
* Stop defaulting `@job_handler_ns` to `nil` in `Queue` (1e4df3e)

/cc @kellyredding
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