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
Change os.sendfile so its arguments are stable #59283
Comments
As I keep saying on python-dev: I think that the argument list for a function should be stable. If you have a function where some abilities are only available on certain platforms, it's best to always accept default no-op parameters for those parameters, rather than adding and removing parameters based on what functionality is available. os.sendfile() accepts either four or seven parameters, depending on the current platform. However, it's a new function in trunk and therefore has no installed base. So it's not too late to change it. I propose to amend os.sendfile so it always accepts all seven arguments. Its signature would therefore be the following, on all platforms: os.sendfile(out, in, offset, nbytes, headers=None, trailers=None, flags=0) Passing in a non-None value for headers or trailers, or a nonzero value for flags, on a platform where the seven-argument form of sendfile is not available would raise NotImplementedError. |
I believe, that instead of a integer flags will be better and more portable to use boolean parameters (diskio=True, wait=True, sync=False). All additional parameters should be keyword-only. os.sendfile(out, in, offset, nbytes, *, headers=None, trailers=None, diskio=True, wait=True, sync=False) |
I probably prefer this. If the original implementers are okay with it then I'd be happy to do it that way. But at the very least I want to get rid of the two function signatures. |
There are some other functions with strange/variable prototype: These Python functions are thin wrapper of the C function, that's why their prototype can be surprising. |
It does not follow that this behavior is desirable, or that it should be emulated in new code. There's a lot of legacy code with unexpected API design that would not pass muster were it proposed today. Consider this: what should the Signature (PEP-362) for fcntl.ioctl look like?
The "thin wrapper" os.sendfile is 130 lines of C. Furthermore I disagree with the idea that os does or should contain "thin wrappers". It strikes me as unbearably provincial thinking among UNIX developers. Anyone who thinks os contains "thin wrappers" should examine the Windows implementation of stat(). But at the heart of the matter, I see no benefit to exposing Python developers to the idiosyncrasies of poor C API design. I feel strongly that one way Python becomes "pythonic" is that it aims for the convenience of the programmer--not the language designer and not the implementer. The Python calling convention is far more flexible than the C calling convention. We should put it to good use here. |
I'm -0 about unifying the function signature and raise NotImplementedError where an arg is not supported. I'm -1 about os.sendfile(..., diskio=True, wait=True, sync=False). In both cases users willing to use these 3 args are already supposed to know what they're doing and that the code they're writing is not portable hence in practical terms I can't see a real gain in making a change. |
I agree. However, I think Martin is a proponent of the "thin wrapper" approach, I personally like the change, except for |
What's wrong with mmap? It uses list of optional arguments ( |
I indeed think that the code is fine as it stands, and no change is needed, and that the proposed changes make matters worse. The point of the thin wrappers approach is that you can read the manpage of your system, and immediately can trust that this is what the Python function will do. It is unfortunate that BSD and Linux have chosen to give the function the same name despite the signature differences, but there is no value in hiding this fact from the Python user. The whole point of this function is performance and zero copy. Anybody using it will need to understand well what they are doing, and that their code is highly system-dependent. If you want cross-platform code, use shutil.copyfileobj. I could agree to a higher-level function that tries to avoid system differences, but that function shouldn't be called sendfile. For example, the socket object could have a sendfd or sendstream method which would use the proper variant of sendfile if available, else uses a regular read/send loop. I always found the name "sendfile" confusing, anyway, since it's not the file that is being sent, but the all (or some) of the contents of the file. |
Of course it does, as the mmap syscall(), since this arguments have nothing to do with one another. If we did this for, let's say, mmap() |
It is usually folly to disagree with MvL, and yet I am about to do just that. As stated, I don't believe in the "thin wrappers" theory of os; where would a Windows programmer find the man page to learn how os.access works on their system? I think os should provide helpful functionality to the programmer, specifically functions that are calling into the system API. But unlike MvL I would *much* rather see consistent functionality across systems than expose the Python programmer to every fiddling nook and cranny of the local API. I prefer "Python" programs to "Python-on-Linux" and "Python-on-Windows" programs. Yet, as much as I'm willing to disagree with MvL, I'm not so sure of myself that I'll check this in against his wishes. If this idea wins more support in the future, we could still make my proposed change to the interface without breaking backwards-compatibility. So I volunteer to table the discussion at least for the 3.3 release. TBH I don't see the benefit of the extra three parameters anyway. headers/trailers is redundant with os.writev, and the three flags are for shaving off cycles that are lost in the noise of the Python interpreter. If I ran the circus I'd remove the latter three parameters and have the same interface everywhere sendfile is available. But I suspect there's no way I can get away with *that* for the 3.3 beta, and once we hit feature freeze I won't be able to remove it. So I guess we're just going to be stuck with them forever. *sigh* -- Let me controversially, hypothetically, propose: if we really want to make raw OS functions available to Python programmers, then so be it, but not in the os module. We could add an "osx" module, and a "linux" module, and a "freebsd" module, and a "win32" module (though I suppose win32all covers that). And we could ditch the whole "rename posix to os" shenanigans. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: