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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for arbitrary sending of messages within multiconn-server.py #112

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

Conversation

pbvillaflores
Copy link

Where to put new files:

  • New files should go into a top-level subfolder, named after the article slug. For example: my-awesome-article

How to merge your changes:

  1. Make sure the CI code style tests all pass (+ run the automatic code formatter if necessary).
  2. Find an RP Team member on Slack and ask them to review & approve your PR.
  3. Once the PR has one positive ("approved") review, GitHub lets you merge the PR.
  4. 馃帀

@pbvillaflores pbvillaflores mentioned this pull request May 21, 2020
@dbader
Copy link
Member

dbader commented May 21, 2020

Can you clarify what this does and how it relates to the code in the tutorial? We typically only merge bug fix PRs since we need to keep the sample code in sync with the tutorial on realpython.com

@pbvillaflores
Copy link
Author

hi dbader, the change made is really just a few lines to make the server code capable of also supporting sending out of arbitrary messages--or at least makes it clearer how this can be achieved. As it stands, you can only easily adapt this code such that the server can send a response when it can reply to an incoming message.

If you try to actually send arbitrary messages from the server app at the moment, it is not so straightforward to do.

I have found other people other than myself have had this struggle. Please see also https://stackoverflow.com/questions/59978318

@dbader
Copy link
Member

dbader commented May 21, 2020

Pinging @natej who's the original author of the tutorial. @natej, it would be good to get your input to see if it makes sense to amend the tutorial or if this is out of scope -- thx :)

@natej
Copy link
Contributor

natej commented May 22, 2020

Hi @dbader and @pbvillaflores This is definitely something that I've seen asked too and a common use case. So I like the idea and appreciate @pbvillaflores working on it.

I share your concern that it may stray a bit and diverge slightly from the tutorial, even though it is slight and only a few lines. I was thinking that it might be better to simply have a separate example in its own file that demonstrates how the original tutorial code could be used and modified. This would keep the original code intact and true to the tutorial content without it needing to be modified. It would also serve as a simple example of how the original code could be modified to suit a different need/behavior/feature. So I think it would serve two goals. Also, if you chose this route and wanted to, I think it would be an easy edit to the tutorial to add a few sentences at the end of the multiconn section pointing out this new file and how it serves as a good example of how to extend the already shown example code for another purpose, i.e. a client connects initially, but then the server sends messages periodically to the client without the client sending any requests.

@pbvillaflores
Copy link
Author

Thanks Nate!

You suggested -- editing the tutorial? Is that something anyone can do?

Oh, btw, what made it difficult for me to work in this mod/enhancement are couple of key points and thus I thought it was worthwhile sending the PR:

  1. The server cannot just send anytime to any client. I.e., the sock_send will fail if you call it with an aribtrary fd.

  2. Why it is that Create SP500.csv聽#1 is true is not clear.

Eventually I realised:

a. mask & selectors.EVENT_READ is not lacking selectors.EVENT_WRITE in order to write.

b. It seems the system just naturally cycles the selector fds and eventually all socket fds become ready to write on a cyclical loop.

I wonder if (b) imposes any extra load on the system.

@natej
Copy link
Contributor

natej commented May 22, 2020

In order for a change to be made, you'd need to get someone from the team to work your proposed changes through the editorial process.

Thanks for pointing out the selector mask change for writes. Yes, you need that for the existing code to work so it recognizes that it can write data to the socket. That's definitely one thing that would make a great addition to the next version of the tutorial: you need to make sure the mask is changed appropriately, depending on what you're doing, so it detects that the socket is ready for reads and/or writes (it's been a while, but maybe I did a decent job of explaining it; I can't remember).

You're also correct that it could impose extra load on the system, i.e. if you don't need to write data to the socket, then there's no reason for your code to get called when the socket is ready for writes.

@pbvillaflores
Copy link
Author

pbvillaflores commented May 25, 2020

Hi Nate, I think it is not true that one needs selectors.EVENT_WRITE in order to actually write.

For example your code example as is, has sending/writing already, and you don't have that setup.

And I have found too that if the code is extended as per my code modification, that it is in fact able to do sending/writing arbitrarily even without selectors.EVENT_WRITE

@natej
Copy link
Contributor

natej commented May 25, 2020

Thanks for letting me know. I'm glad it's working for you in practice. It may be that in another app or platform or under different conditions that it may behave differently. I just thought I'd mention it and suggest reading through the docs on the selector mask and any other sources or articles you can find about it.

@pbvillaflores
Copy link
Author

pbvillaflores commented May 27, 2020

Hi Nate, I've done a bit of googling on the subject and there doesn't seem to be a lot of good answers on this question. I have also tested my code on both windows and ubuntu platforms. Both manifest same behaviour--there is no need to add SELECTORS.EVENT_WRITE in order for the server to write or send. I suppose it doesn't hurt to add SELECTORS.EVENT_WRITE to the mask but it also has no effect.

@pbvillaflores
Copy link
Author

Oh, and I found another person with the same issue with the current code - https://stackoverflow.com/questions/53045592/python-non-blocking-sockets-using-selectors/62034205#62034205

@natej
Copy link
Contributor

natej commented May 27, 2020

They are difficult to find. I know what you mean. Thanks for finding and sharing the link.

This answer from your link is a good one and explains why you're able to write without changing the mask. So you can write, or attempt to write, to the socket without regard for the current socket's status or the state of its write buffer/queue, i.e. you can ignore whether the selector reports that the socket is ready for writing. However, if you do that, be prepared that your code may throw an exception when the write buffer is full. I think you'll see a "would block" exception. See the "Reference" section for BlockingIOError.

Read the "Multi-Connection Server" section again and notice how accept_wrapper() handles getting the client connection/socket conn and it sets the selector to selectors.EVENT_READ | selectors.EVENT_WRITE for it. So when the server writes to the client, it'll only do this when the socket is "ready" for writing.

@pbvillaflores
Copy link
Author

ah. I see you set it when you perform accept()

@pbvillaflores
Copy link
Author

It also looks like the system does a loop cycle of the available selectors and gives them a set chance to run. If you do it out of turn, some sort of error is thrown.

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

3 participants