-
Notifications
You must be signed in to change notification settings - Fork 20
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
Message serializations #28
base: master
Are you sure you want to change the base?
Conversation
@@ -1 +1,2 @@ | |||
/vendor/* | |||
/.idea/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be defined in user's global gitignore. Each user uses own tools.
I've been thinking a lot about this issue and I dont see how this could be solved without introducing serializer interface for a message body. @Sevavietl what do you think? |
@makasim I do not know if it worth to be so strict on message body itself. Maybe we can loosen it a bit like it is done in this PR, but control it with the serializer in context, for example. Like it is done in rdkafka enqueue implementation. Do you see any caveats in such approach? I will look deeply into the JMS, maybe find there some direction. It is hard to get the answer from the top of my head. Let's come back to this later. |
Than a serializer is bound to transport, and if you switch from one to another the code will break. |
I'd say it's user's responsibility to select and attach correct serializer. If one want to shoot himself in the foot by using different serialization method for producing and consuming, then it's his fault for doing so. |
@makasim, @Steveb-p, I am also against making serializer bound to specific transport. It is better to resolve it from configurations: di container is a must-have nowadays. So I see it as transport, message, and serializer that is configured for the specific message/transport combo. This way there is no coupling between the serializer and message, serializer and transport. And as a plus, the same serializer can be used across the project, without the need for adapters and wrappers. Well, there is one adapter to make serializer to satisfy enqueue internal serializer, as there is no psr for the serializer. |
{ | ||
return $this->body; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please insert newline and make git happy :)
EDIT: The same goes for the test file for it.
In my opinion, since this is an interface only, there should be nothing about how Just the fact that a Attaching As I see it: What's the purpose of the trait that is added in PR? |
What is the state of this PR? @Sevavietl will you continue on it, or you don't have the time? |
ref #27