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

Integer responseID without nonBlocking support #53

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Integer responseID without nonBlocking support #53

wants to merge 1 commit into from

Conversation

10urshin
Copy link

No description provided.

* will be re-thrown. If non-blocking is false, an exception will not be thrown in the calling thread but instead can
* be retrieved with {@link #waitForLastResponse()} or {@link #waitForResponse(byte)}, similar to a return value. */
public void setTransmitExceptions (boolean transmit);

Copy link
Member

Choose a reason for hiding this comment

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

We lose non-blocking, transmit return value, and transmit exceptions? I'd rather not lose the functionality, especially non-blocking.

@10urshin
Copy link
Author

Well I told you I have deleted it cause I wont use it and you asked a pull request so here it is.

Later i might limit the integer and add nonblocking back.What I am thinking about is a fixed size ConcurrentHashMap which blocks when it is full and can be used for fixing this unexpected behavior even with using byte for responseID.

@NathanSweet
Copy link
Member

Aye, sorry I should have mentioned that I'd rather not remove features unless there is good reason. Your PR is still useful for the other bits. If you'd like to add back the functionality and keep your other changes great, if not the PR can stay until myself or someone else gets to doing it.

Not sure we'd want to block if no responseIDs are available. Using byte for responseID isn't terribly important, using a varint is fine if it is needed. A separate, single byte could be used for various flags (eg exceptions) instead of cramming them into the responseID.

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

Successfully merging this pull request may close these issues.

2 participants