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

Added FlowControl to Protocol #51

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

TarasLevelUp
Copy link

Now we can wait for writes if TCP buffers are full. This can be very useful for Publisher applications.
NOTE: not too sure if adding an option to publish is that good of an API. Any ideas?

@tvoinarovskyi tvoinarovskyi force-pushed the add_protocol_flow_control branch 4 times, most recently from b021fa2 to 9ff538c Compare October 4, 2015 18:22
@tvoinarovskyi
Copy link
Contributor

Must probably be fixed based on #56

@JustinTArthur
Copy link

@TarasLevelUp I think exposing buffer flow control hints is great—thanks for getting the ball rolling. As for adding it as an option to publish, I don't quite like the idea.

One problem is using the word "flush" in the naming/docs. It gives the idea that the publish has been flushed to the system's networking stack and this isn't necessarily true. We might have flushed the buffer below the high watermark, but some of our publish might still be in the buffer unflushed.

Secondly, having two different return types for the same API method feels wrong.

What about exposing/documenting that drain coroutine you added to the sender object? If we don't document sender anywhere, we could have a coroutine on the connection object that calls it.

Something like

@asyncio.coroutine
def publish_numbers():
    connection = yield from asynqp.connect('localhost', 5672, username='guest', password='guest')
    channel = yield from connection.open_channel()
    exchange = yield from channel.declare_exchange('test.exchange', 'direct')

    for number in range(100):
    	msg = asynqp.Message({'myNumber': number})
    	
    	yield from connection.drain()
    	exchange.publish(msg, 'routing.key')

I don't like the term "drain" either, but it does match watch asyncio and nodejs call it :)

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.

4 participants