Skip to content

Conversation

@jendakol
Copy link
Collaborator

PullResult instead of Option for RabbitMQPullConsumer
ConversionException to api module

Sorry for so big PR however it's cause mainly by moving ConversionException into another package (and module).

PullResult instead of Option for RabbitMQPullConsumer
ConversionException to api module
@jendakol jendakol self-assigned this May 21, 2018
@jendakol jendakol requested review from augi and petr-k May 21, 2018 16:12
Copy link
Member

@augi augi left a comment

Choose a reason for hiding this comment

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

  1. Remove the parsingFailureAction completely.
  2. The consumer action should accept a sealed trait instead of plain Delivery, so users were able to handle the parsing failure.

README.md Outdated

#### Consumer parsing failure

It may happen the delivery is not parsable as your `MyClass`. It won't be passed into the consumer and `ParsingFailureAction` will be used
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer to have something like PullResult as input to regular consumer (created via newConsumer). So use a sealed trait instead of plain Delivery. I believe the code would be more concise.

README.md Outdated

#### Pull-Consumer parsing failure

It may happen the delivery is not parsable as your `MyClass`. `ParsingFailureAction` will be used for handling such message and you will get
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer to remove this feature - it's not IMHO required because you can achieve the same behavior just by processing the PullResult.MalformedContent when calling pull. You have to handle this case anyway...

jendakol added 3 commits May 22, 2018 17:32
removed ParsingFailureAction
PullResult removed MalformedContent
@jendakol
Copy link
Collaborator Author

Updated. The Delivery is now either Delivery.Ok or Delivery.MalformedContent. @augi

Migration-5-6.md Outdated
1. The API now uses type-conversions - provide type and related converter when creating producer/consumer.
See [related section](README.md#providing-converters-for-producer/consumer) in docs.
1. The `Delivery` is now generic - e.g. `Delivery[Bytes]` (depends on type-conversion).
1. The `Delivery` is now sealed trait - there are `Delivery.Ok[A]` (e.g. `Delivery[Bytes]` ,depends on type-conversion) and `Delivery.MalformedContent`.
Copy link
Member

Choose a reason for hiding this comment

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

Invalid spacing around ,

@jendakol jendakol merged commit 361bc1c into master May 25, 2018
@jendakol jendakol deleted the ParsingFailureAction branch May 25, 2018 12:39
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.

3 participants