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

Wishlist for the next major revision #276

Closed
bronson opened this issue Feb 5, 2016 · 2 comments
Closed

Wishlist for the next major revision #276

bronson opened this issue Feb 5, 2016 · 2 comments

Comments

@bronson
Copy link
Contributor

bronson commented Feb 5, 2016

For discussion, this is a list of potential improvements to Queue Classic. Most of them are breaking changes that require a major revision bump. Would be nice to do them all at once.

  • Discourage inheriting from QC classes (using inheritance makes it harder to make backward-compatible changes in the future). Try to satisfy everyone with callbacks and configuration instead.
  • If it's unreasonable to remove inheritance entirely, reduce the exposed API by marking parts of classes private and declaring explicit base classes. Right now, with everything public, most modifications are technically breaking changes.
  • Minimum of Ruby 2 / Rails 4
  • Rename exposed methods and rake tasks to be more consistent. (example: add Worker#work_off and rake qc:work_off. #256)
  • Change schema to make monitoring easier: Proposed schema changes #252
  • Simplify creating QC::Queue objects: Proposed interface change to QC::Queue #249
  • Ensure migrations don't break if I remove QC from my gemfile (will update when I file the PR)
  • Maybe remove forking? Honest question: Is forking supposed to work for 3.0.0? #207 (comment))
  • Maybe just use ActiveRecord's connection? Since AR allows multiple connections, does QC need its own implementation?

very unlikely:

Anything else? Anyone have opinions?

I'd like to expand any of these that make sense to PRs.

@bronson
Copy link
Contributor Author

bronson commented Feb 6, 2016

I played with the second-to-last item in the list: removing the QC::ConnAdapter class and just using ActiveRecord's connection (or a plain PGconn when AR isn't available).

 lib/queue_classic.rb              |  27 ++++++++--
 lib/queue_classic/conn_adapter.rb | 111 --------------------------------------
 lib/queue_classic/worker.rb       |  21 +++++---
 test/helper.rb                    |   1 +
 4 files changed, 39 insertions(+), 121 deletions(-)

So far it's a nice simplification. I still need to make tests pass again but it's getting there. Some assumptions:

  • mutex synchronization isn't needed. This seems OK to me because Rails and AR are concurrent now. Parallelization should produce a nice speedup I'd have to do a torture test to see if this is still correct.
  • Proposed interface change to QC::Queue #249 is applied. Breaking changes everywhere.

Result

I'm not sure if it's worth pursuing further...

This change removes a bunch of code and makes things nicer for people using AR/ActiveJob (especially in the migrations!). However, it also removes some functionality from the standalone case (mostly stuff that AR already does like logging and exception handling).

Personally, I only use QC in Rails apps sharing the AR connection, so this cleanup appeals to me a lot. But if people care about preserving all QC's functionality when used standalone then, I gotta say, this doesn't make much sense.

@bronson
Copy link
Contributor Author

bronson commented Mar 7, 2016

Closing because our project is now using Que. Still, queue_classic is a great project. Hope it all goes well.

@bronson bronson closed this as completed Mar 7, 2016
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

No branches or pull requests

1 participant