-
Notifications
You must be signed in to change notification settings - Fork 434
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
Supporting protobuf message support #1329
base: master
Are you sure you want to change the base?
Conversation
LGTM, though ci is red |
c6d9e15
to
ba5fd89
Compare
These 2 specs |
@makasim The one which are failed are not because of my changes. Can we re-run them or merge this PR? |
I cannot merge failing CI it should be green. |
I know but can you re-run those? Because all the other same version specs are passing :) plus they are 'gpqm' related specs. |
@ahmedkhan847 I can help look later to see if we figure out something for the failed tests. |
@makasim Can you rerun it I have fixed the tests for |
@makasim the issue was the docker image tag for localstack. I have updated the docker image tag to the previous then latest version in docker-compose file and now all the specs are passing on my local. I couldn't find how your ci if it is using the docker-compose file it will automatically pick it and should work. |
@makasim this is the log of specs from my local
|
@makasim Can you please review it? |
@makasim fix the issue in the waitfor part. I hope it works this time :) Sorry it taking a lot of your time as well :) |
Because of the serializer in my current project, I was unable to get messages in protobuf format. So instead encoding/decoding every message from JSON we can send the actual message as it is so that it can be serialized into the appropriate class for that message. Protobuf PHP provides the methods:
$message->fromString()
and$message->fromJson()
which can bind the data to your class. Also added attributes as properties of the message.