Skip to content

Conversation

jordansjones
Copy link
Contributor

Instead of .FullName.

@Horusiath
Copy link
Contributor

Horusiath commented Jul 10, 2016

Any rationale for that? Problem with assembly qualified names is that they contain things like version or token number - which are not used elsewhere in serialization pipeline and can cause unexpected behavior when incremental upgrades are going to happen while pub/sub is working.

@alexvaluyskiy
Copy link
Contributor

I think we should have an extension like this and use it everywhere, instead of AssemblyQualifiedName

    public static class TypeExtensions
    {
        public static string TypeQualifiedNameForManifest(this Type type)
        {
            return type == null ? string.Empty : $"{type.FullName}, {type.Assembly.GetName().Name}";
        }
    }

@cconstantin
Copy link
Contributor

@alexvaluyskiy iirc correctly I added such an extension when updating persistence a while back, but it may be in persistence.

@JordanJones
Copy link

@Horusiath the rational is that without the assembly name, Type.GetType(...) returns null. But I'm all for having an extension method.

I'll create one if there isn't one already in the core Akka project and update my PR either tonight or tomorrow sometime.

@jordansjones
Copy link
Contributor Author

Turns out that I missed the TypeQualifiedNameForManifest method on the base serializer. I've opted to use that instead of the AssemblyQualifiedName value.

@jordansjones jordansjones changed the title Use AssemblyQualifiedName with Manifest requiring Serializers DistributedPubSub: Use TypeQualifiedNameForManifest with Manifest required Serializers Jul 13, 2016
@alexvaluyskiy alexvaluyskiy merged commit 79c49e6 into akkadotnet:dev Jul 13, 2016
@Aaronontheweb Aaronontheweb added this to the 1.1.1 milestone Jul 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants