Skip to content

Update _socket to use the socket2 crate; similar to the C sockets api #1572

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

Merged
merged 8 commits into from
Nov 2, 2019

Conversation

coolreader18
Copy link
Member

Also adds socket.py from CPython 3.6.0

@coolreader18 coolreader18 force-pushed the coolreader18/socket-socket2 branch 6 times, most recently from 3e5b4c4 to 116aa9c Compare October 27, 2019 03:02
@palaviv
Copy link
Contributor

palaviv commented Oct 27, 2019

@coolreader18 can you split this PR into smaller ones? It will be easier to review that way

@coolreader18
Copy link
Member Author

@palaviv #1574

@coolreader18 coolreader18 force-pushed the coolreader18/socket-socket2 branch 3 times, most recently from a2954e6 to ec93629 Compare October 28, 2019 17:55
@coolreader18 coolreader18 force-pushed the coolreader18/socket-socket2 branch from ec93629 to 460e149 Compare October 29, 2019 03:29
Copy link
Contributor

@palaviv palaviv left a comment

Choose a reason for hiding this comment

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

Looks good. Just a few comments added. I hoped we would be able to use the std::net connection but this is way simpler.

}
fn flush(&mut self) -> io::Result<()> {
Ok(())
Ok(vm.ctx.new_tuple(vec![fd, addr_tuple]))
Copy link
Contributor

Choose a reason for hiding this comment

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

You can return here (fd, addr_tuple)

let mut buffer = vec![0u8; bufsize];
let addr = match self.sock().recv_from(&mut buffer) {
Ok((_, addr)) => addr,
Err(ref e) if e.kind() == io::ErrorKind::TimedOut => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest adding a function name convert_socket_error that will check if this is a timeout and then call convert_io_error. That will remove a lot of duplication.

@coolreader18 coolreader18 force-pushed the coolreader18/socket-socket2 branch from 8b6fabd to e563945 Compare November 1, 2019 22:42
@coolreader18 coolreader18 merged commit 24cc679 into master Nov 2, 2019
@palaviv
Copy link
Contributor

palaviv commented Nov 2, 2019

@coolreader18 can you look at #1580? The windows test of select failed there.

@coolreader18 coolreader18 deleted the coolreader18/socket-socket2 branch November 2, 2019 18:49
@coolreader18
Copy link
Member Author

I'm not sure, that's a really weird failure. I tried rerunning it but the checkout failed because the branch is deleted.

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.

2 participants