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

Split up the driver interface #224

Closed
sagikazarmark opened this issue Mar 5, 2016 · 6 comments
Closed

Split up the driver interface #224

sagikazarmark opened this issue Mar 5, 2016 · 6 comments

Comments

@sagikazarmark
Copy link
Contributor

In my opinion, there is too much responsibility in the Driver interface. For example not all queues allow to give information about itself (which can't be abstracted anyway), not all of them allow to create new or remove queues, etc. Therefore I think we should split up the Driver interface into several smaller one:

  • The core driver interface should allow push/pop/ack which are necessary for the messaging itself
  • The should be a driver supporting information retrieval (info, peek?, maybe separately?)
  • There should be a driver which allows listing queues
  • There should be a driver which allows creating/removing queues

This of course comes with complications in the implementation, however I think it would be better to put knowsledge (what a driver capable of) into abstraction instead of returning empty/default values. If a driver is not capable of something, why would you try to do that thing?

Looking forward to hear your feedback.

@sagikazarmark
Copy link
Contributor Author

@henrikbjorn may I ask for your feedback on this one?

@sagikazarmark
Copy link
Contributor Author

sagikazarmark commented Oct 28, 2016

Separate driver responsibilities:

  • Basic/Messaging: push/pop/ack messages
  • Queue management: list/create/remove queue (might be automatically done by the broker)
  • Counting messages in a queue (might belong to the previous one)
  • Peeking
  • Stats

These are all separate responsibilities and not all of the drivers support all of them. A default noop implementation might make sense and might be required in many cases, but instead of relying on meaningless empty values, we could rely on the driver's nature (namely implements and interface of not).

@sagikazarmark
Copy link
Contributor Author

@henrikbjorn WDYT?

@henrikbjorn
Copy link
Contributor

Not sure this is worth pursuing, currently the drivers are "internal" only, meaning the end user should not work with them directly.

@henrikbjorn
Copy link
Contributor

but if it will clean up some code paths i am for it.

@sagikazarmark
Copy link
Contributor Author

Well, after rethinking there is only one remaining thing that bugs me: it seems to be wrong to return an empty array in peek when the driver does not support peeking.

No-op code (like createQueue when manual creation is not necessary) is not that big problem after all.

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

2 participants