Navigation Menu

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

Add block=True back to get_msg() #641

Merged
merged 1 commit into from Apr 12, 2021

Conversation

davidbrochart
Copy link
Member

@davidbrochart davidbrochart merged commit 9080886 into jupyter:master Apr 12, 2021
@davidbrochart davidbrochart deleted the fix_get_msg branch April 12, 2021 19:51
@kevin-bates
Copy link
Member

I was going to comment on whether or not we should deprecate the block parameter at this time (and remove it in 7.0) and/or document that timeout is only used when block == True (although the comment was the same previously). Kind of an odd API and agree that the timeout value serves that purpose just fine. We should probably default its value to 0 once block is removed (IMO).

@davidbrochart
Copy link
Member Author

Ah, sorry for merging so quickly. Actually, now that we decided to go for a major release and it's still a pre-release, we could remove it.
But defaulting timeout to 0 means we just poll for a message, which is the opposite of what we have now by default (we wait until we have a message)?

@kevin-bates
Copy link
Member

But defaulting timeout to 0 means we just poll for a message, which is the opposite of what we have now by default (we wait until we have a message)?

I see. I haven't looked at the socket poll() method, but now see what you're saying where None essentially means wait until we have a message. Yeah, that should probably remain the default then.

@blink1073 blink1073 added this to the 7.0 milestone Jul 30, 2021
@kevin-bates
Copy link
Member

@davidbrochart - was there still follow-up work for the 7.0 release on this? I was under the impression the block parameter would be removed but see it's still there. Is this just a matter of removing the parameter and leaving timeout defaulted to None? (Essentially make the change that was backed out of the 6.x releases?)

@davidbrochart
Copy link
Member Author

I think we can get rid of the block parameter, since we can do anything with timeout only. I opened #671.

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

Successfully merging this pull request may close these issues.

None yet

3 participants