-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Add backward compatibility to PrimitiveSerializers #5280
Add backward compatibility to PrimitiveSerializers #5280
Conversation
@@ -78,6 +85,9 @@ public override object FromBinary(byte[] bytes, string manifest) | |||
/// <inheritdoc /> | |||
public override string Manifest(object obj) | |||
{ | |||
if (_useNeutralPrimitives) | |||
return obj.GetType().TypeQualifiedName(); |
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.
Need to emulate the old serializer behaviour, which is to embed the type name as manifest.
Related code:
https://github.com/akkadotnet/akka.net/blob/dev/src/core/Akka.Remote/MessageSerializer.cs#L62-L74
… github.com:Arkatufus/akka.net into Add_backward_compatibility_for_PrimitiveSerializers
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.
LGTM - please send in a second PR with reference documentation in HOCON for this
{ | ||
var c = RARP.For(Sys).Provider.RemoteSettings.Config.GetConfig("akka.actor.serialization-settings.primitive"); | ||
c.Should().NotBeNull(); | ||
c.GetBoolean("use-neutral-primitives").Should().BeFalse(); |
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.
LGTM
|
||
namespace Akka.Remote.Tests.Serialization | ||
{ | ||
public class BugFix5279Spec: AkkaSpec |
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.
LGTM
Add backward compatibility to PrimitiveSerializers (akkadotnet#5280) * Add backward compatibility to PrimitiveSerializers * Update API Approver list
* Change the base class of PrimitiveSerializers to SerializerWithStringManifest (#4989) * Change the base class of PrimitiveSerializers to SerializerWithStringManifest * Add backward compatibility to the wire format * Update API Approver list (cherry picked from commit 25246ac) * cherry-picked from 6101fea Add backward compatibility to PrimitiveSerializers (#5280) * Add backward compatibility to PrimitiveSerializers * Update API Approver list * Change PrimitiveSerializer compatibility switch setting name from `use-neutral-primitives` to `use-legacy-behavior` for less ambiguity (#5290) * Change PrimitiveSerializer compatibility switch setting name from `use-neutral-primitives` to `use-legacy-behavior` for less ambiguity * Fix unit test * Change default to full compatibility (on) * Fix unit test Co-authored-by: Aaron Stannard <aaron@petabridge.com> (cherry picked from commit 302e3cb) * Code cleanup * Fix FSharp.Core package problem * Fix CI/CD script - Change vmimage to windows-latest and ubuntu-latest * Fix CI/CD script - Install SDK * Bump net45 target to net452 * Fix CI/CD script * Bump Incrementalist.Cmd to 0.9.0 * Revert "Bump net45 target to net452" This reverts commit 2d0d76d. * Revert "Code cleanup" This reverts commit d72b753. * Code cleanup * Code cleanup
Closes #5279