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

Change ISubscription and adding EOF messages into stream #42

Closed

Conversation

AndreSteenbergen
Copy link
Contributor

No description provided.

@AndreSteenbergen AndreSteenbergen changed the title Change ISubscription, addind EOF messages into stream Change ISubscription and adding EOF messages into stream Aug 14, 2018
@Horusiath
Copy link
Contributor

From what I've seen, there's no notion of EOF in JVM - instead they start from getting end offset at the beginning and after reaching it emitting appropriate event type. See akka.streams.eventsourcing for more inspiration.

…'s an EOF message. Splitted classes into separate files.
@AndreSteenbergen
Copy link
Contributor Author

@Horusiath I couldn't find an endOffset in the metadata. I guess the java client is a little richer then the c# client?

This way a user can await all messages being send.
original CompletionSource is well suited for this.
The code had a few async callbacks. These callbacks made sure the stage never completed. Changed calls to use callback
It isn't always important to get deliveryreports. The async retrieval can make the flow slow, it needs to wait for the ACKs for the messages sent based on number of threads.
@Danthar
Copy link
Member

Danthar commented Jan 8, 2019

Anything holding this PR back ?

@AndreSteenbergen
Copy link
Contributor Author

AndreSteenbergen commented Jan 8, 2019

Actually... I have a local version using the beta 2 instead of the experimental Kafka connector. This PR would work as is, no problem.

But I can update this PR with a newer kafka client and later Akka.Net version. I think I can have a fresh PR done by tomorrow 1300 CET. That is better I guess.

@Danthar
Copy link
Member

Danthar commented Jan 9, 2019

An update would be great. 👍 Drop a comment here once you pushed your changes, and ill take a look.

@AndreSteenbergen
Copy link
Contributor Author

Done; I am using this code in production without issues so far. Only thing I have doubts about are the failstage parts. Kafka has a connection reaper, every 10 minutes the connection will be dropped. Raising an error, but the the producer is still able to push messages to kafka without restarting. Don't know what to do really.

@Danthar
Copy link
Member

Danthar commented Jan 11, 2019

We fixed build issues. Needs rebase

@Danthar
Copy link
Member

Danthar commented Jan 14, 2019

@AndreSteenbergen would you mind also updating the version prefix in the csproj for kafka ?

@Danthar Danthar mentioned this pull request Jan 14, 2019
@AndreSteenbergen
Copy link
Contributor Author

will do.

@AndreSteenbergen
Copy link
Contributor Author

I am in the process of updating all nuget packages for kafka (again) kafka 1.0.0beta3 has quite a number of differences and breaking changes. wrt beta2, 1.0.0 is supposed to be out in march. If there is a huge demand for beta3 I can make a push, but I need to alter the tests as well.

@Aaronontheweb
Copy link
Member

@Arkatufus is this PR still necessary?

@AndreSteenbergen
Copy link
Contributor Author

nope; there is a better nuget package already in Akka Streams

@Aaronontheweb
Copy link
Member

Ah! I didn't realize this was for Kafka - good deal

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.

None yet

4 participants