-
Notifications
You must be signed in to change notification settings - Fork 69
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
fix: fix tests generation logic #1049
Conversation
This includes: 1) Fix test logic for grpc+rest case, when clients with both transports need to be initialized in parametrized tests 2) Fix 100% coverage problem for rest clients, when the http error (>= 400 error code) case logic was not covered.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One non-blocking question, otherwise LGTM.
{% endif %} | ||
{% endfor %} | ||
request = request_type(request_init) | ||
{% if method.client_streaming %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is streaming supported for the rest transport?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not yet; that is intended to be part of next sprint. Currently it just raises NotImplemented
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Just one question: how the hell did you get around the transport __call__
patch problem? I had to do this massive rewrite of the rest layer, but you seem to have avoided it.
This includes:
This should conclude the rest transport related fixes (for the time being at least)