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

Refactor waiter handling in channel #83

Closed
wants to merge 1 commit into from

Conversation

foolswood
Copy link
Contributor

Rough pass at tidying up waiter handling.

@dzen
Copy link
Contributor

dzen commented Feb 26, 2016

Are you using the no_wait argument with aioamqp ?

@dzen
Copy link
Contributor

dzen commented Feb 26, 2016

beware of the typo on_wait in your commit message :p

@foolswood
Copy link
Contributor Author

I am not using no_wait, it was just the visual noise in the code (and the open issue) prompting a friday afternoon tidy.

@foolswood foolswood changed the title Refactor on_wait in channel Refactor waiter handling in channel Feb 26, 2016
@dzen
Copy link
Contributor

dzen commented Feb 26, 2016

Happy to know we have a few tests ;)

@foolswood
Copy link
Contributor Author

I was leaning on them for this, they did their job, let's see if that fix makes things any better.

@foolswood
Copy link
Contributor Author

Seems fine now, fixed the typo whilst at it.

@@ -188,21 +188,32 @@ def server_channel_close(self, frame):
self.connection_closed(results['reply_code'], results['reply_text'])

@asyncio.coroutine
def _write_frame_awaiting_response(
self, waiter_id, frame, request, no_wait, **kwargs):
Copy link
Contributor

Choose a reason for hiding this comment

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

This indentation seems really odd. Since there are lines with lenght longer than 80, we should keep this code style

@foolswood
Copy link
Contributor Author

I believe this is ready now.

@dzen
Copy link
Contributor

dzen commented Mar 4, 2016

Yes, squash both commits and I'll merge it

@foolswood
Copy link
Contributor Author

They're squashed.

@dzen
Copy link
Contributor

dzen commented Mar 11, 2016

It's merged since yesterday.

@dzen dzen closed this Mar 11, 2016
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