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

Fixes for bugs found during manual use #3170

Merged
merged 1 commit into from
Sep 1, 2015

Conversation

nathanielmanistaatgoogle
Copy link
Member

This is forward progress toward #3153.

(1) In _ingestion, it's the "details" attribute of a
NoSuchMethodException that we want. The "message" is inherited from the
base Exception class.

(2) In _transmission, use a proper sum type for representing operation
abortion. Trying to overload the existing _completion value for
status-and-details-when-aborting was trying to be too clever.

(3) In _calls... oof. Just look. Oof. Test coverage for this code path
is added.

(4) In _service, the application-provided
face.MultiMethodImplementation isn't directly callable, but rather
exposes a method named "service".

(5) In crust.implementations, the wrapping that we've put around the
application-provided face.MultiMethodImplementation *is* directly
callable, and *does not* expose a method named "service".

(6) Also in crust.implementations, base.NoSuchMethodError's constructor
takes a code value and a details value.

(7) Again in crust.implementations, the application-provided
face.MultiMethodImplementation may be None, and if it is None, we
shouldn't wrap it with an adaptation function that would only raise a
TypeError at a later time.
@nathanielmanistaatgoogle
Copy link
Member Author

Python is green.

@nicolasnoble
Copy link
Member

Is this okay to merge ? Otherwise this will need to be re-done on the beta branch.

@nathanielmanistaatgoogle
Copy link
Member Author

This is ready for merge.

@@ -246,7 268,9 @@ def advance(self, initial_metadata, payload, completion, allowance):

def timeout(self, timeout):
"""See _interfaces.TransmissionManager.timeout for specification."""
if self._transmitting:
if self._abort.kind is not _Abort.Kind.NOT_ABORTED:
Copy link
Contributor

Choose a reason for hiding this comment

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

There's probably a nicer way to phrase this w/o double negatives, but nothing comes immediately to mind, so, meh.

soltanmm added a commit that referenced this pull request Sep 1, 2015
Fixes for bugs found during manual use
@soltanmm soltanmm merged commit 924b67d into grpc:master Sep 1, 2015
@lock lock bot locked as resolved and limited conversation to collaborators Jan 30, 2019
@lock lock bot unassigned soltanmm Jan 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants