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

Inconsistent APIs for IO::Socket::Async and Proc::Async #3707

Closed
p6rt opened this issue Feb 27, 2015 · 6 comments
Closed

Inconsistent APIs for IO::Socket::Async and Proc::Async #3707

p6rt opened this issue Feb 27, 2015 · 6 comments

Comments

@p6rt
Copy link

p6rt commented Feb 27, 2015

Migrated from rt.perl.org#123949 (status was 'resolved')

Searchable as RT123949$

@p6rt
Copy link
Author

p6rt commented Feb 27, 2015

From @moritz

Both IO​::Socket​::Async and Proc​::Async have the options to get supplies
for binary or string data, but there are two inconsitentcies​:

1) Proc​::Async has methods stdout(​:$bin), IO​::Socket​::Async has
chars_supply and bytes_supply. There should be some naming consistency here

2) Proc​::Async has live supplies, and requires you to set them up before
starting the process. IO​::Socket​::Async has on-demand supplies that you
can get after the fact.

In the interest of a smooth asynchronous experience, we should make them
more consistent.

@p6rt
Copy link
Author

p6rt commented Feb 27, 2015

From @lizmat

On 27 Feb 2015, at 17​:24, Moritz Lenz (via RT) <perl6-bugs-followup@​perl.org> wrote​:

# New Ticket Created by Moritz Lenz
# Please include the string​: [perl #​123949]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl6/Ticket/Display.html?id=123949 >

Both IO​::Socket​::Async and Proc​::Async have the options to get supplies
for binary or string data, but there are two inconsitentcies​:

1) Proc​::Async has methods stdout(​:$bin), IO​::Socket​::Async has
chars_supply and bytes_supply. There should be some naming consistency here

2) Proc​::Async has live supplies, and requires you to set them up before
starting the process. IO​::Socket​::Async has on-demand supplies that you
can get after the fact.

In the interest of a smooth asynchronous experience, we should make them
more consistent.

I will take that into the newio branch, unless someone beats me to it in nom.

Liz

@p6rt
Copy link
Author

p6rt commented Feb 27, 2015

The RT System itself - Status changed from 'new' to 'open'

@p6rt
Copy link
Author

p6rt commented Mar 4, 2015

From @moritz

Hi,

On 27.02.2015 23​:57, Elizabeth Mattijsen wrote​:

On 27 Feb 2015, at 17​:24, Moritz Lenz (via RT) <perl6-bugs-followup@​perl.org> wrote​:

# New Ticket Created by Moritz Lenz
# Please include the string​: [perl #​123949]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl6/Ticket/Display.html?id=123949 >

Both IO​::Socket​::Async and Proc​::Async have the options to get supplies
for binary or string data, but there are two inconsitentcies​:

1) Proc​::Async has methods stdout(​:$bin), IO​::Socket​::Async has
chars_supply and bytes_supply. There should be some naming consistency here

2) Proc​::Async has live supplies, and requires you to set them up before
starting the process. IO​::Socket​::Async has on-demand supplies that you
can get after the fact.

In the interest of a smooth asynchronous experience, we should make them
more consistent.

I will take that into the newio branch, unless someone beats me to it in nom.

Thanks.

While I'm at it, there are two more inconsistencies between
IO​::Socket​::Async and IO​::Socket​::INET​:

1. IO​::Socket​::Async.connect() uses positional arguments for the host
and port, while IO​::Socket.​:INET.new uses

2. :​:Async uses separate .connect and .listen methods, :​:INET uses a
.new with a separate :listen argument to distinguish them. (I think I
tend to prefer the separate methods approach).

Cheers,
Moritz

@p6rt
Copy link
Author

p6rt commented Nov 27, 2015

From @jnthn

Hi,

I wrote these issues up in a gist​:

https://gist.github.com/jnthn/9d8b2e22882d7f7a871e

They got some discussion on channel, and the decisions implemented.

On Wed Mar 04 01​:08​:29 2015, moritz wrote​:

On 27.02.2015 23​:57, Elizabeth Mattijsen wrote​:

On 27 Feb 2015, at 17​:24, Moritz Lenz (via RT) <perl6-bugs-
followup@​perl.org> wrote​:
Both IO​::Socket​::Async and Proc​::Async have the options to get
supplies
for binary or string data, but there are two inconsitentcies​:

1) Proc​::Async has methods stdout(​:$bin), IO​::Socket​::Async has
chars_supply and bytes_supply. There should be some naming
consistency here

Now IO​::Socket​::Async is consistent with IO​::Handle​: you have a .Supply method and can give it an optional :bin parameter. This is consistent with the :$bin parameter that stdout and stdin have on Proc​::Async. Since there are two supplies you could want, then we can't just have .Supply on a Proc​::Async. Updated S32-io/IO-Socket-Async.t to use the .Supply method.

2) Proc​::Async has live supplies, and requires you to set them up
before
starting the process. IO​::Socket​::Async has on-demand supplies that
you
can get after the fact.

When you are listening on a socket for incoming connections, you already have the socket, so there's no chance to take the supply earlier. With processes, obtaining the supplies has a secondary role of indicating what handles to bind when the process is spawned, which has to happen before we spawn it. So, the current design seems right.

While I'm at it, there are two more inconsistencies between
IO​::Socket​::Async and IO​::Socket​::INET​:

1. IO​::Socket​::Async.connect() uses positional arguments for the host
and port, while IO​::Socket.​:INET.new uses

2. :​:Async uses separate .connect and .listen methods, :​:INET uses a
.new with a separate :listen argument to distinguish them. (I think I
tend to prefer the separate methods approach).

IO​::Socket​::INET gets to keep its new method, but it now also has connect and listen convenience methods, taking positional arguments, for consistency with IO​::Socket​::Async. Added tests in S32-io/IO-Socket-INET.t for new listen and connect methods.

/jnthn

@p6rt
Copy link
Author

p6rt commented Nov 27, 2015

@jnthn - Status changed from 'open' to 'resolved'

@p6rt p6rt closed this as completed Nov 27, 2015
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

No branches or pull requests

1 participant