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

Driver interface changes #377

Open
sagikazarmark opened this issue Feb 21, 2018 · 4 comments
Open

Driver interface changes #377

sagikazarmark opened this issue Feb 21, 2018 · 4 comments

Comments

@sagikazarmark
Copy link
Contributor

Cleanup

As mentioned in #224 I believe the Driver interface needs a little bit of cleanup.

Currently it's not really an abstraction, and part of the problem is this also reaches the Queue abstraction layer.

Talking about three methods in particular:

  • count (also in Queue)
  • peek (also in Queue)
  • info

These are never used in the consumer-producer context (which I think is the primary goal of Bernard), I believe they are there for the sake of Juno.

  • Count: it's the least problematic of all, but still not supported by some drivers. I would either move it to a CountableDriver interface or implement Countable instead and check for those interfaces in Juno.
  • Peek: Most of the drivers doesn't even support this. I would do the same with peek as with count. I think it makes the Driver interface more stable and other drivers easier to implement.
  • Info: This is probably the most problematic one, as it returns totally unstructured data. I don't really know what the best solution would be here, but at least some structuring would be nice. Eg. return an array of key value pairs at least.

Other changes

I also suggest the following changes:

  • Implement rejectMessage as suggested in Consider implementing Driver::rejectMessage() #134
  • Implement some kind of RawMessage object which exposes the raw message and the receipt instead of how currently pop returns messages. It's too easy this way to return invalid data.

If you are comfortable with these changes I'm more than happy to do the changes and move Bernard towards a stable version.

@sagikazarmark sagikazarmark added this to the 1.0.0-beta1 milestone Feb 21, 2018
@acrobat
Copy link
Member

acrobat commented Feb 22, 2018

I like the idea of moving stuff the a CountableDriverInterface and other specific interfaces so each driver can decided on what it want to support of "extra features". This way we also keep the general interface clean

@sagikazarmark
Copy link
Contributor Author

That's the idea.

@acrobat
Copy link
Member

acrobat commented Feb 22, 2018

I would say go for it! 💪

@sagikazarmark
Copy link
Contributor Author

sagikazarmark commented Feb 22, 2018

I've done some research (previous assumptions were purely based on the driver interface and it's implementations): looks like it would be quite hard to make peek and count removed from the interface, but at the same time kept as an optional feature.

I'm not really happy to say this, but without major refactor on the queue level as well removing those methods is not possible.

While I think it would be great if we could remove those things from the interface, I believe this part of the library is mature enough to be ready for a first stable.

As @henrikbjorn said: the driver concept is an internal thing, end users (devs) should not meet with the interface at all.

I will do some more research, but it looks like it would be easier for us (at least for now) to leave them be.

As for the other changes: I still think those should be done, but I wonder if they should block releasing 1.0.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants