Overhaul Python C layer #1558
Overhaul Python C layer #1558
Conversation
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. |
|
... and part of cleaning up the commit history will include setting the proper e-mail address. |
| @@ -43,4 +43,4 @@ class BlockingInvocationInlineServiceTest( | |||
|
|
|||
|
|
|||
| if __name__ == '__main__': | |||
| unittest.main() | |||
| unittest.main(verbosity=2) | |||
There was a problem hiding this comment.
I've never before seen "verbosity=2" in a call to "unittest.main()". What does it do?
There was a problem hiding this comment.
Changes the real-time normal test output to be the test names rather than .s and Ss and Es - kinda nice when the code segfaults and Python's interpreter never gets to perform error handling. It might be prudent for me to move these little verbosity changes to a separate commit, maybe add more of them.
|
It appears that the segfault isn't present when building with CONFIG=dbg but is present when CONFIG=opt. Fun stuff. |
|
What's the reason for having large and few C files rather than small and many? Might I push gently to move in the small-and-many direction? |
|
I did that so that I could have all the functions be declared Besides that, fixed the segfault... but I don't see Travis running in the checks. Any ideas why that might be? (edit: it was because the commits in this PR conflicted with |
soltanmm
force-pushed
the
extension
branch
2 times, most recently
from
2f86b17
to
4ed0abc
Compare
|
CLAs look good, thanks! |
soltanmm
force-pushed
the
extension
branch
3 times, most recently
from
e5630e0
to
7c7d671
Compare
| @@ -1,3 +1,4 @@ | |||
| /* vim: set expandtab shiftwidth=2 tabstop=2 softtabstop=2: */ | |||
There was a problem hiding this comment.
Please drop these lines?
There was a problem hiding this comment.
Werp - I'd forgotten I had those there.
|
I believe I've covered my error-handling-bases and have dropped @Yhg1s a line - we'll see where this PR ends up in the next few days. I guess, too, that we're sticking with C89 for the foreseeable future. Also, at the risk of inadvertently invoking any number of corollaries of Murphy's law, I'll note that I've yet to see one of our ever-present Travis failures out of this PR... EDIT: Murphy is alive and well. |
| @@ -31,21 +31,31 @@ | |||
| * | |||
| */ | |||
|
|
|||
| #ifndef _ADAPTER__SERVER_H_ | |||
There was a problem hiding this comment.
"src/python/src/grpc/_adapter/_server.h → src/python/src/grpc/_adapter/_c/module.c" is a funny move to make, and makes for a funny diff - how easily could you change this file to being a fork of something more sensible (_c.c?)?
There was a problem hiding this comment.
I'll look into git mv, but IIRC it's just the equivalent of git add + git rm.
There was a problem hiding this comment.
git autodetects moves... there's no user control over this.
|
(Good work, by the way.) |
|
I managed to introduce a segfault on local testing. EDIT: Might just be because I trashed my git repository state on this particular machine... |
|
It was a trashed local repo state. |
|
cc #529, as this makes that issue outdated. |
|
On Fri, May 22, 2015 at 2:00 AM, Nathaniel Manista <notifications@github.com
Thomas Wouters thomas@python.org Hi! I'm an email virus! Think twice before sending your email to help me |
|
@Yhg1s ping |
Exposes the C core batch API to the Python layers. Provides a shim to enable the old Python API to remain the same (for now).
|
Updated PR to keep up with protobuf upgrades and the pattern set by #1803... |
|
Merging this; it's waited long enough and we can always do a post-commit review later. |
Overhaul Python C layer.
Change the Python C layer to better match the core; currently adapts the new C layer via
_low.pyto what the old_low.pylooked like (sans a name changeEXPIRED->DEADLINE_EXCEEDEDto match the core enums). This effectively moves the boundary between old-style and new-style APIs from the shared Python/C layer to the lowest pure Python layer.There is much to be cleaned up in this pull request (file copyrights, doc comments, commit messages, source file organization, enum location [lingual]...), and I'd rather wait for #1472 and #1493 to go through first as they seem like potential sources of surprise (which is also a reason for the alternative, but, meh).
This is being put up as a PR to check against Travis (all tests are passing locally) and for initial comments.