Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Don't generate serializers for foreign types in Orleans.Streaming.EventHubs #4487

Merged
merged 2 commits into from
Apr 24, 2018
Merged

Don't generate serializers for foreign types in Orleans.Streaming.EventHubs #4487

merged 2 commits into from
Apr 24, 2018

Conversation

ReubenBond
Copy link
Member

No description provided.

@ReubenBond ReubenBond added this to the 2.0.3 milestone Apr 18, 2018
jason-bragg
jason-bragg reviewed Apr 23, 2018
View reviewed changes
src/Azure/Orleans.Streaming.EventHubs/Properties/AssemblyInfo.cs Outdated

[assembly: InternalsVisibleTo("ServiceBus.Tests")]

[assembly: KnownAssembly(typeof(EventHubSequenceTokenV2), TreatTypesAsSerializable = true)]
Copy link
Contributor

Choose a reason for hiding this comment

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

How is a serializer generated for EventHubSequenceTokenV2? It's not used in any grain interfaces. That's why it was previously marked with the GenerateSerializer attribute

Copy link
Member Author

Choose a reason for hiding this comment

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

It's marked as [Serializable]

@ReubenBond
Copy link
Member Author

ReubenBond commented Apr 23, 2018 •
edited

@jason-bragg I added GenerateSerializer for EventHubSequenceTokenV2 as an extra safety net: the build will fail if no serializer is generated. In addition, we also check that a test to drive confidence: InternalSerializableTypesHaveSerializers checks that a serializer exists at runtime for EventHubSequenceTokenV2

EDIT: oh, I see the issue: that type has a hand-coded serializer, so no serializer will be generated for it (correct behavior). I'll add the attribute to another type.

@jason-bragg
Copy link
Contributor

@dotnet-bot test functional please

@jason-bragg jason-bragg merged commit 57b9852 into dotnet:master Apr 24, 2018
@ReubenBond ReubenBond deleted the fix-eh-codegen branch April 25, 2018 02:59
sergeybykov pushed a commit to sergeybykov/orleans that referenced this pull request May 5, 2018
@ReubenBond @sergeybykov
Don't generate serializers for foreign types in Orleans.Streaming.Eve…
b1e34d0 
…ntHubs (dotnet#4487)

* Don't generate serializers for foreign types in Orleans.Streaming.EventHubs

* Add GenerateSerializer(typeof(EventHubSequenceToken))
ReubenBond added a commit that referenced this pull request May 9, 2018
@ReubenBond
Don't generate serializers for foreign types in Orleans.Streaming.Eve…
76e818c 
…ntHubs (#4487)

* Don't generate serializers for foreign types in Orleans.Streaming.EventHubs

* Add GenerateSerializer(typeof(EventHubSequenceToken))
@github-actions github-actions bot locked and limited conversation to collaborators Dec 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jason-bragg jason-bragg jason-bragg approved these changes

Assignees

@jason-bragg jason-bragg

Labels
None yet
Projects
None yet
Milestone
2.0.3
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@ReubenBond @jason-bragg