close
Skip to content

Support for akka 2.2.1#169

Merged
johnynek merged 5 commits into
twitter:developfrom
2m:akka221
Dec 23, 2013
Merged

Support for akka 2.2.1#169
johnynek merged 5 commits into
twitter:developfrom
2m:akka221

Conversation

@2m
Copy link
Copy Markdown
Contributor

@2m 2m commented Dec 16, 2013

Hey,

are you guys thinking about Akka 2.2.1? This is a quick patch which makes chill compile against akka 2.2.1, but not akka 2.1.4.

If you are not planning to upgrade soon, maybe I should think of backwards compatible solution.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extending things in the akka package? really needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea. Serialization.currentTransportInformation is private to the akka package. On the second thought this probably means that it should not be accessed from outside and ActorRef should be serialized in some other way. I will investigate that.

@2m
Copy link
Copy Markdown
Contributor Author

2m commented Dec 18, 2013

Found a more clean solution. It seems that akka team provided ActorRef serializer, which is now beeing used in the fix.

@2m
Copy link
Copy Markdown
Contributor Author

2m commented Dec 19, 2013

Yea... Forgot to actually write to Output. Fixed that.

@ianoc
Copy link
Copy Markdown
Contributor

ianoc commented Dec 19, 2013

Given that sort of typo had a clean build, maybe some tests that things RT through the kryo properly?

@2m
Copy link
Copy Markdown
Contributor Author

2m commented Dec 20, 2013

Added a test which puts ActorRef through serialization and deserialization.

@johnynek
Copy link
Copy Markdown
Contributor

Looks good! Thanks!

johnynek added a commit that referenced this pull request Dec 23, 2013
Support for akka 2.2.1
@johnynek johnynek merged commit 1027600 into twitter:develop Dec 23, 2013
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

Successfully merging this pull request may close these issues.

3 participants