-
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
fix Akka.Serialization.Hyperion dependency #3003
Conversation
fa85115
to
059b58a
Compare
@@ -70,7 +70,8 @@ public HyperionSerializer(ExtendedActorSystem system, HyperionSerializerSettings | |||
preserveObjectReferences: settings.PreserveObjectReferences, | |||
versionTolerance: settings.VersionTolerance, | |||
surrogates: new[] { akkaSurrogate }, | |||
knownTypes: provider.GetKnownTypes())); | |||
knownTypes: provider.GetKnownTypes(), | |||
ignoreISerializable:true)); |
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.
IMHO, I'm fine with leaving this setting on. ISerializable
is dangerous and shouldn't be used for anything other than local serialization, not cross-network.
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.
@Aaronontheweb But we don't have any alternatives for Exceptions. We can use only ISerializable
serialization
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.
old behavior would be to ignore it because ISerializable support in Hyperion was introduced only in latest release.
Maybe you can just release a fixed version for "Akka.Serialization.Hyperion" without ISerialization support and discuss this afterwards ?
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.
I agree, this PR is not about using ignoreISerializable or not. Its about fixing the package. Default value is false, so set it at false. And lets have the discussion on when to turn this option on, a different time.
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.
old behavior would be to ignore it because ISerializable support in Hyperion was introduced only in latest release.
Agreed. We weren't using it before. Status quo.
cc @Ralf1108 - this will fail to compile if the build server is the issue.