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

IPC Implementation! #84

Merged
merged 4 commits into from
Oct 13, 2020
Merged

IPC Implementation! #84

merged 4 commits into from
Oct 13, 2020

Conversation

TheButlah
Copy link
Contributor

@TheButlah TheButlah commented Oct 13, 2020

Rebased on #83, might want to diff against that for review.

First alternative transport type! This PR implements the IPC transport with unix sockets. Because the underlying IO was abstracted out in #77, this was fairly straightforward and required only adding in the necessary ipc module in transport, as well as a representation for the IPC endpoint.

@TheButlah
Copy link
Contributor Author

TheButlah commented Oct 13, 2020

One issue I'm getting with this is that the socket file that the UnixStream creates doesn't get deleted immediately - if you immediately re-bind to that same location, you often get an error. It would therefore be good to either add some retry logic in the bind() call, or figure out a way to block the destructor/unbind call of the socket until the file is closed.

However, I'd like to ask to do that in a later PR, once the current PR chain is resolved. I don't want to open any more without merging some first, for fear of painful rebases.

@Alexei-Kornienko Alexei-Kornienko merged commit aa6bfee into zeromq:master Oct 13, 2020
@TheButlah
Copy link
Contributor Author

@Alexei-Kornienko Thank you so much for your attentiveness and reviews! I know time is precious so I especially appreciate your help and responsiveness in getting all these PRs done :)

Out of curiosity, is me submitting several co-dependent PRs and then rebasing them all each time one of them merges the best way to do things? Should I be bulk-merging them somehow?

@TheButlah TheButlah deleted the ipc branch October 13, 2020 23:55
@Alexei-Kornienko
Copy link
Collaborator

@TheButlah reviewing small requests is much simpler. I understand that it can be annoying to you to rebase them all the time. I can try to review big requests implementing some big features but in such case I will take more time for review because it requires at least couple of hours to understand what/why is changed. I can only do this on weekends cause I'm busy with other stuff and don't have enough free time to work on it.

@TheButlah
Copy link
Contributor Author

@Alexei-Kornienko if what I've been doing so far works for you and makes it manageable, i'll keep doing that. No worries!

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

2 participants