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

Use Try for ConsumerMessage value #133

Merged
merged 1 commit into from
Feb 3, 2020

Conversation

gmethvin
Copy link
Collaborator

@gmethvin gmethvin commented Jan 30, 2020

Allow ConsumerMessage instances to be able to represent a Pulsar message whose value cannot be parsed. This brings the behavior roughly in line with the Java client, which will allow you to create messages with invalid data but will throw when getValue is called. It also prevents the Akka streams source from failing immediately upon creating the ConsumerMessage, allowing the user code to decide how to handle the error.

@gmethvin gmethvin requested a review from sksamuel January 30, 2020 03:43
@gmethvin gmethvin force-pushed the consumer-message-try branch 2 times, most recently from 3bed06f to 128695d Compare January 30, 2020 06:28
@gmethvin gmethvin force-pushed the consumer-message-try branch from 128695d to d6037b8 Compare January 30, 2020 19:30
@gmethvin
Copy link
Collaborator Author

I also went ahead and reverted some library versions based on our discussion in #127.

@@ -7,15 +7,15 @@ travisBuildNumber in Global := sys.env.getOrElse("TRAVIS_BUILD_NUMBER", "0")
def travisVersion(v: String, tb: String): String = v.stripSuffix("-SNAPSHOT") + s".$tb-SNAPSHOT"

val org = "com.sksamuel.pulsar4s"
val AkkaStreamVersion = "2.6.1"
val AkkaStreamVersion = "2.5.28" // compatible with Akka 2.5.x and 2.6.x
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -18,6 +18,8 @@ trait ConsumerMessage[T] {

def value: T

def valueTry = Try(value)
Copy link
Contributor

Choose a reason for hiding this comment

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

valueSafe ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I slightly prefer the Try naming since it's clear what it is. There are many reasons why something might be "safe" or "unsafe".

@gmethvin gmethvin merged commit 952114a into CleverCloud:master Feb 3, 2020
@gmethvin gmethvin deleted the consumer-message-try branch February 3, 2020 05:56
@sksamuel
Copy link
Contributor

sksamuel commented Feb 3, 2020 via email

@gmethvin
Copy link
Collaborator Author

gmethvin commented Feb 3, 2020

@sksamuel can we do a new release for this?

@sksamuel
Copy link
Contributor

sksamuel commented Feb 3, 2020 via email

@gmethvin
Copy link
Collaborator Author

gmethvin commented Feb 4, 2020

alright thanks!

@sksamuel
Copy link
Contributor

sksamuel commented Feb 4, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants