Re: [Python-Dev] Best Python API for exposing posix_spawn

[Pablo Galindo Salgado]

Thank you everyone that commented in this thread about the best interface
for posix_spawn. I have finished implementing Antoine's suggestion in the
PR:

https://github.com/python/cpython/pull/5109

I think it would be good if we can have this merged before the feature lock
at the end of the month if possible.

Thanks you very much everyone for your time and suggestions!

On Wed, 10 Jan 2018, 09:17 Pablo Galindo Salgado, 
wrote:

> I think I really like Antoine's suggestion so I'm going to finish
> implementing it that way. I think this keeps the API simple, does not bring
> in the os module new dependencies, keeps the C implementation clean and is
> consistent with the rest of the posix module. I will post an update when is
> ready.
>
> Thank you everyone for sharing your view and advice!
>
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Best Python API for exposing posix_spawn

[Pablo Galindo Salgado]

I think I really like Antoine's suggestion so I'm going to finish
implementing it that way. I think this keeps the API simple, does not bring
in the os module new dependencies, keeps the C implementation clean and is
consistent with the rest of the posix module. I will post an update when is
ready.

Thank you everyone for sharing your view and advice!
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Best Python API for exposing posix_spawn

[Brett Cannon]

On Tue, 9 Jan 2018 at 02:42 Nick Coghlan  wrote:

> On 9 January 2018 at 20:01, Antoine Pitrou  wrote:
> > On Mon, 08 Jan 2018 09:11:38 +
> > Pablo Galindo Salgado  wrote:
> >> Hi,
> >>
> >> I'm currently working on exposing posix_spawn in the posix module (and
> by
> >> extension in the os module). You can find the initial implementation in
> >> this PR:
> >>
> >> https://github.com/python/cpython/pull/5109
> >>
> >> As pointed out by Gregory P. Smith, some changes are needed in the way
> the
> >> file_actions arguments is passed from Python. For context, posix_spawn
> has
> >> the following declaration:
> >>
> >> int posix_spawn(pid_t *pid, const char *path,
> >> const posix_spawn_file_actions_t *file_actions,
> >> const posix_spawnattr_t *attrp,
> >> char *const argv[], char *const envp[]);
> >>
> >> Here, file_actions is an object that represents a list of file actions
> >> (open, close or dup2) that is populated using helper functions on the C
> API.
> >>
> >> The question is: what is the best way to deal with this argument?
> >
> > How about a list of tuples like:
> > [(os.SPAWN_OPEN, 4, 'README.txt', os.O_RDONLY, 0),
> >  (os.SPAWN_CLOSE, 5),
> >  (os.SPAWN_DUP2, 3, 6),
> >  ]
> >
> > I don't expect this API to be invoked directly by user code so it
> > doesn't have to be extremely pretty.
>
> I'll note that one advantage of this approach is that it ties in well
> with how the C API is going to have to deal with it anyway: a switch
> statement dispatching on the first value, and then passing the
> remaining arguments to the corresponding posix_file_actions API.
>

Plus the posix module tends to stick reasonably close to the C API anyway
since it's such a thin wrapper.


>
> Wrapping it all up in a more Pythonic self-validating API would then
> be the responsibility of the subprocess module (in the standard
> library), or third party modules.
>

+1 from me on Antoine's suggestion. Might as well keep it simple.
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Best Python API for exposing posix_spawn

[Nick Coghlan]

On 9 January 2018 at 20:01, Antoine Pitrou  wrote:
> On Mon, 08 Jan 2018 09:11:38 +
> Pablo Galindo Salgado  wrote:
>> Hi,
>>
>> I'm currently working on exposing posix_spawn in the posix module (and by
>> extension in the os module). You can find the initial implementation in
>> this PR:
>>
>> https://github.com/python/cpython/pull/5109
>>
>> As pointed out by Gregory P. Smith, some changes are needed in the way the
>> file_actions arguments is passed from Python. For context, posix_spawn has
>> the following declaration:
>>
>> int posix_spawn(pid_t *pid, const char *path,
>> const posix_spawn_file_actions_t *file_actions,
>> const posix_spawnattr_t *attrp,
>> char *const argv[], char *const envp[]);
>>
>> Here, file_actions is an object that represents a list of file actions
>> (open, close or dup2) that is populated using helper functions on the C API.
>>
>> The question is: what is the best way to deal with this argument?
>
> How about a list of tuples like:
> [(os.SPAWN_OPEN, 4, 'README.txt', os.O_RDONLY, 0),
>  (os.SPAWN_CLOSE, 5),
>  (os.SPAWN_DUP2, 3, 6),
>  ]
>
> I don't expect this API to be invoked directly by user code so it
> doesn't have to be extremely pretty.

I'll note that one advantage of this approach is that it ties in well
with how the C API is going to have to deal with it anyway: a switch
statement dispatching on the first value, and then passing the
remaining arguments to the corresponding posix_file_actions API.

Wrapping it all up in a more Pythonic self-validating API would then
be the responsibility of the subprocess module (in the standard
library), or third party modules.

Cheers,
Nick.

-- 
Nick Coghlan   |   ncogh...@gmail.com   |   Brisbane, Australia
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Best Python API for exposing posix_spawn

[Antoine Pitrou]

On Mon, 08 Jan 2018 09:11:38 +
Pablo Galindo Salgado  wrote:
> Hi,
> 
> I'm currently working on exposing posix_spawn in the posix module (and by
> extension in the os module). You can find the initial implementation in
> this PR:
> 
> https://github.com/python/cpython/pull/5109
> 
> As pointed out by Gregory P. Smith, some changes are needed in the way the
> file_actions arguments is passed from Python. For context, posix_spawn has
> the following declaration:
> 
> int posix_spawn(pid_t *pid, const char *path,
> const posix_spawn_file_actions_t *file_actions,
> const posix_spawnattr_t *attrp,
> char *const argv[], char *const envp[]);
> 
> Here, file_actions is an object that represents a list of file actions
> (open, close or dup2) that is populated using helper functions on the C API.
> 
> The question is: what is the best way to deal with this argument?

How about a list of tuples like:
[(os.SPAWN_OPEN, 4, 'README.txt', os.O_RDONLY, 0),
 (os.SPAWN_CLOSE, 5),
 (os.SPAWN_DUP2, 3, 6),
 ]

I don't expect this API to be invoked directly by user code so it
doesn't have to be extremely pretty.

Regards

Antoine.


___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Best Python API for exposing posix_spawn

[Nick Coghlan]

On 9 January 2018 at 17:07, Serhiy Storchaka  wrote:
> 09.01.18 05:31, Nick Coghlan пише:
>> As with DirEntry, I don't see any obvious value in making the new
>> objects iterable though - we should be able to just use named field
>> access in both the C and Python APIs.
>
> Do you suggest to add a class corresponding to posix_spawn_file_actions_t
> with methods corresponding to posix_spawn_file_* functions?

Sorry, I should have said explicitly that I liked Greg's suggestion of
modeling this as an iterable-of-objects at the Python layer - I was
just agreeing with Brett that those should be objects with named
attributes, rather than relying on tuples with a specific field order.

That way a passed in list of actions would look something like one of
the following:

# Three distinct classes, "FileActions" namespace for disambiguation
[os.FileActions.Close(0),
 os.FileActions.Open(1, '/tmp/mylog', os.O_WRONLY, 0o700),
 os.FileActions.Dup2(1, 2),
]

# Single class, three distinct class constructor methods
[os.FileAction.close(0),
 os.FileAction.open(1, '/tmp/mylog', os.O_WRONLY, 0o700),
 os.FileAction.dup2(1, 2),
]

While my initial comment supported having 3 distinct classes, writing
it out that way pushes me more towards having a single type where the
constructor names match the relevant module function names.

Cheers,
Nick.

-- 
Nick Coghlan   |   ncogh...@gmail.com   |   Brisbane, Australia
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Best Python API for exposing posix_spawn

[Serhiy Storchaka]

09.01.18 01:05, Gregory P. Smith пише:
On Mon, Jan 8, 2018 at 12:36 PM Serhiy Storchaka > wrote:


08.01.18 11:11, Pablo Galindo Salgado пише:
 > Following Gregory's comment on the PR I understand that he is
proposing
 > to have three objects in the os module representing each action
and pass
 > a sequence of these objects to the Python API. What I am not sure
about
 > this is that there is no previous example of such classes in the os
 > module for other similar APIs and therefore I am not sure if
there is a
 > better approach.

I would pass a sequence like:

[(os.close, 0),
   (os.open, 1, '/tmp/mylog', os.O_WRONLY, 0o700),
   (os.dup2, 1, 2),
]


i agree with just a list of tuples, but i suggest creating namedtuple 
instances in the posix module for the purpose (one each for close, dup2, 
open) .  Don't put a reference to a function in the tuple as Serhiy 
suggested as, while obvious what it means, it gives the wrong impression 
to the user: nothing is calling the Python functions.  This is a posix 
API that takes a list of arguments for a specific set of system calls 
for _it_ to make for us in a specific order.


Creating three new classes has higher cost than creating three 
singletones. There are some advantages of using existing functions as tags.


But this is not the only possible interface. If there is a single order 
of actions (first close, then open, finally dup2), actions can be 
specified as three keyword-only arguments taking sequences of integers 
or tuples:


posix_spawn(..., close=[0],
open=[(1, '/tmp/mylog', os.O_WRONLY, 0o700)],
dup2=[(1, 2)])

But this perhaps is not able to express all useful cases.

___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Best Python API for exposing posix_spawn

[Serhiy Storchaka]

09.01.18 05:31, Nick Coghlan пише:

On 8 January 2018 at 19:11, Pablo Galindo Salgado  wrote:

Following Gregory's comment on the PR I understand that he is proposing to
have three objects in the os module representing each action and pass a
sequence of these objects to the Python API. What I am not sure about this
is that there is no previous example of such classes in the os module for
other similar APIs and therefore I am not sure if there is a better
approach.


Probably the closest prior art would be the os.DirEntry objects used
for the items yielded from os.scandir - that is the same general idea
(a dedicated Python class to represent a C struct), just in the other
direction.

As with DirEntry, I don't see any obvious value in making the new
objects iterable though - we should be able to just use named field
access in both the C and Python APIs.


Do you suggest to add a class corresponding to 
posix_spawn_file_actions_t with methods corresponding to 
posix_spawn_file_* functions?


class posix_spawn_file_actions:
def __init__(self): ...
def addclose(self, fildes): ...
def addopen(self, fildes, path, oflag, mode): ...
def adddup2(self, fildes, newfildes): ...
def destroy(self): pass
__del__ = destroy

This maximally corresponds the C API. But doesn't look Pythonic.

___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Best Python API for exposing posix_spawn

[Nick Coghlan]

On 8 January 2018 at 19:11, Pablo Galindo Salgado  wrote:
> Following Gregory's comment on the PR I understand that he is proposing to
> have three objects in the os module representing each action and pass a
> sequence of these objects to the Python API. What I am not sure about this
> is that there is no previous example of such classes in the os module for
> other similar APIs and therefore I am not sure if there is a better
> approach.

Probably the closest prior art would be the os.DirEntry objects used
for the items yielded from os.scandir - that is the same general idea
(a dedicated Python class to represent a C struct), just in the other
direction.

As with DirEntry, I don't see any obvious value in making the new
objects iterable though - we should be able to just use named field
access in both the C and Python APIs.

Cheers,
Nick.

-- 
Nick Coghlan   |   ncogh...@gmail.com   |   Brisbane, Australia
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Best Python API for exposing posix_spawn

[Brett Cannon]

On Mon, 8 Jan 2018 at 15:06 Gregory P. Smith  wrote:

> On Mon, Jan 8, 2018 at 12:36 PM Serhiy Storchaka 
> wrote:
>
>> 08.01.18 11:11, Pablo Galindo Salgado пише:
>> > Following Gregory's comment on the PR I understand that he is proposing
>> > to have three objects in the os module representing each action and pass
>> > a sequence of these objects to the Python API. What I am not sure about
>> > this is that there is no previous example of such classes in the os
>> > module for other similar APIs and therefore I am not sure if there is a
>> > better approach.
>>
>> I would pass a sequence like:
>>
>> [(os.close, 0),
>>   (os.open, 1, '/tmp/mylog', os.O_WRONLY, 0o700),
>>   (os.dup2, 1, 2),
>> ]
>>
>
> i agree with just a list of tuples, but i suggest creating namedtuple
> instances in the posix module for the purpose (one each for close, dup2,
> open) .
>

I a namedtuple really necessary for this versus a simple object? There is
no backwards-compatibility here with an old tuple-based interface so
supporting both tuples and named access doesn't seem necessary to me.

-Brett


>   Don't put a reference to a function in the tuple as Serhiy suggested as,
> while obvious what it means, it gives the wrong impression to the user:
> nothing is calling the Python functions.  This is a posix API that takes a
> list of arguments for a specific set of system calls for _it_ to make for
> us in a specific order.
>
> -gps
>
>
>>
>> ___
>> Python-Dev mailing list
>> Python-Dev@python.org
>> https://mail.python.org/mailman/listinfo/python-dev
>>
> Unsubscribe:
>> https://mail.python.org/mailman/options/python-dev/greg%40krypto.org
>>
> ___
> Python-Dev mailing list
> Python-Dev@python.org
> https://mail.python.org/mailman/listinfo/python-dev
> Unsubscribe:
> https://mail.python.org/mailman/options/python-dev/brett%40python.org
>
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Best Python API for exposing posix_spawn

[Gregory P. Smith]

On Mon, Jan 8, 2018 at 4:03 PM Random832  wrote:

> On Mon, Jan 8, 2018, at 18:05, Gregory P. Smith wrote:
> > i agree with just a list of tuples, but i suggest creating namedtuple
> > instances in the posix module for the purpose (one each for close, dup2,
> > open) .  Don't put a reference to a function in the tuple as Serhiy
> > suggested as, while obvious what it means, it gives the wrong impression
> to
> > the user: nothing is calling the Python functions.  This is a posix API
> > that takes a list of arguments for a specific set of system calls for
> _it_
> > to make for us in a specific order.
>
> Instead of a sequence of functions to call, it'd be nice if a higher-level
> API could allow just passing in a mapping of file descriptor numbers to
> what they should point to in the new process, and the implementation
> figures out what sequence is necessary to get that result.
>
> And at that point we could just extend the subprocess API to allow
> redirection of file descriptors other than 0/1/2, and have an
> implementation of it in terms of posix_spawn.
>

sure, but high level APIs don't belong in the os/posix module.

-gps
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Best Python API for exposing posix_spawn

[Random832]

On Mon, Jan 8, 2018, at 18:05, Gregory P. Smith wrote:
> i agree with just a list of tuples, but i suggest creating namedtuple
> instances in the posix module for the purpose (one each for close, dup2,
> open) .  Don't put a reference to a function in the tuple as Serhiy
> suggested as, while obvious what it means, it gives the wrong impression to
> the user: nothing is calling the Python functions.  This is a posix API
> that takes a list of arguments for a specific set of system calls for _it_
> to make for us in a specific order.

Instead of a sequence of functions to call, it'd be nice if a higher-level API 
could allow just passing in a mapping of file descriptor numbers to what they 
should point to in the new process, and the implementation figures out what 
sequence is necessary to get that result.

And at that point we could just extend the subprocess API to allow redirection 
of file descriptors other than 0/1/2, and have an implementation of it in terms 
of posix_spawn.
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Best Python API for exposing posix_spawn

[Gregory P. Smith]

On Mon, Jan 8, 2018 at 12:36 PM Serhiy Storchaka 
wrote:

> 08.01.18 11:11, Pablo Galindo Salgado пише:
> > Following Gregory's comment on the PR I understand that he is proposing
> > to have three objects in the os module representing each action and pass
> > a sequence of these objects to the Python API. What I am not sure about
> > this is that there is no previous example of such classes in the os
> > module for other similar APIs and therefore I am not sure if there is a
> > better approach.
>
> I would pass a sequence like:
>
> [(os.close, 0),
>   (os.open, 1, '/tmp/mylog', os.O_WRONLY, 0o700),
>   (os.dup2, 1, 2),
> ]
>

i agree with just a list of tuples, but i suggest creating namedtuple
instances in the posix module for the purpose (one each for close, dup2,
open) .  Don't put a reference to a function in the tuple as Serhiy
suggested as, while obvious what it means, it gives the wrong impression to
the user: nothing is calling the Python functions.  This is a posix API
that takes a list of arguments for a specific set of system calls for _it_
to make for us in a specific order.

-gps


>
> ___
> Python-Dev mailing list
> Python-Dev@python.org
> https://mail.python.org/mailman/listinfo/python-dev
> Unsubscribe:
> https://mail.python.org/mailman/options/python-dev/greg%40krypto.org
>
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Best Python API for exposing posix_spawn

[Serhiy Storchaka]

08.01.18 11:11, Pablo Galindo Salgado пише:
Following Gregory's comment on the PR I understand that he is proposing 
to have three objects in the os module representing each action and pass 
a sequence of these objects to the Python API. What I am not sure about 
this is that there is no previous example of such classes in the os 
module for other similar APIs and therefore I am not sure if there is a 
better approach.


I would pass a sequence like:

[(os.close, 0),
 (os.open, 1, '/tmp/mylog', os.O_WRONLY, 0o700),
 (os.dup2, 1, 2),
]

___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Best Python API for exposing posix_spawn

[Brett Cannon]

On Mon, 8 Jan 2018 at 07:57 Pablo Galindo Salgado 
wrote:

> Hi,
>
> I'm currently working on exposing posix_spawn in the posix module (and by
> extension in the os module). You can find the initial implementation in
> this PR:
>
> https://github.com/python/cpython/pull/5109
>
> As pointed out by Gregory P. Smith, some changes are needed in the way the
> file_actions arguments is passed from Python. For context, posix_spawn has
> the following declaration:
>
> int posix_spawn(pid_t *pid, const char *path,
> const posix_spawn_file_actions_t *file_actions,
> const posix_spawnattr_t *attrp,
> char *const argv[], char *const envp[]);
>
> Here, file_actions is an object that represents a list of file actions
> (open, close or dup2) that is populated using helper functions on the C API.
>
> The question is: what is the best way to deal with this argument?
>
> Following Gregory's comment on the PR I understand that he is proposing to
> have three objects in the os module representing each action and pass a
> sequence of these objects to the Python API. What I am not sure about this
> is that there is no previous example of such classes in the os module for
> other similar APIs and therefore I am not sure if there is a better
> approach.
>
> Thanks you very much for your time!
>

Any chance of using an enum?
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

[Python-Dev] Best Python API for exposing posix_spawn

[Pablo Galindo Salgado]

Hi,

I'm currently working on exposing posix_spawn in the posix module (and by
extension in the os module). You can find the initial implementation in
this PR:

https://github.com/python/cpython/pull/5109

As pointed out by Gregory P. Smith, some changes are needed in the way the
file_actions arguments is passed from Python. For context, posix_spawn has
the following declaration:

int posix_spawn(pid_t *pid, const char *path,
const posix_spawn_file_actions_t *file_actions,
const posix_spawnattr_t *attrp,
char *const argv[], char *const envp[]);

Here, file_actions is an object that represents a list of file actions
(open, close or dup2) that is populated using helper functions on the C API.

The question is: what is the best way to deal with this argument?

Following Gregory's comment on the PR I understand that he is proposing to
have three objects in the os module representing each action and pass a
sequence of these objects to the Python API. What I am not sure about this
is that there is no previous example of such classes in the os module for
other similar APIs and therefore I am not sure if there is a better
approach.

Thanks you very much for your time!

Pablo Galindo
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com