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.

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 ackerror serialization/deserialization #270

Merged
merged 3 commits into from
Jul 20, 2022

Conversation

zachary-rote
Copy link
Collaborator

No description provided.

@github-actions github-actions bot added the bug Something isn't working label Jul 18, 2022
@@ -171,6 +175,18 @@ class PersistentActorSpec
}

"PersistentActor" should {
"Serialize and Deserialize AckError" in {
val error = ACKError(new Exception("test-failure"))
Copy link
Contributor

Choose a reason for hiding this comment

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

You can test this through the Akka serializer as well (which is what actually gets used). We have a few of those in this suite actually. Looking a little closer it looks like maybe the compareStringResults bit from those might be hiding some issues with serialization

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@codecov-commenter
Copy link

Codecov Report

Merging #270 (330a8bf) into main (71de420) will increase coverage by 0.55%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #270      +/-   ##
==========================================
+ Coverage   59.92%   60.48%   +0.55%     
==========================================
  Files         153      153              
  Lines        3244     3244              
  Branches       91      104      +13     
==========================================
+ Hits         1944     1962      +18     
+ Misses       1300     1282      -18     
Impacted Files Coverage Δ
...a/surge/internal/persistence/PersistentActor.scala 93.00% <ø> (+0.69%) ⬆️
...health/windows/actor/HealthSignalWindowActor.scala 92.48% <0.00%> (-1.51%) ⬇️
.../surge/internal/kafka/KafkaProducerActorImpl.scala 77.87% <0.00%> (+0.60%) ⬆️
...StreamsUpdatePartitionsOnStateChangeListener.scala 91.42% <0.00%> (+8.57%) ⬆️
.../surge/kafka/streams/KafkaStreamManagerActor.scala 65.82% <0.00%> (+12.65%) ⬆️
...windows/stream/actor/HealthSignalStreamActor.scala 60.00% <0.00%> (+26.66%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7111840...330a8bf. Read the comment docs.

@jeffboutotte jeffboutotte merged commit 631466b into main Jul 20, 2022
@jeffboutotte jeffboutotte deleted the fix/ackerror_deserialization branch July 20, 2022 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants