fix: Assorted fixes and tweaks to improve legibility and functionality. #80
Conversation
| Args: | ||
| body: Body text to replace the current body of the incomming request. | ||
| clear_body: If true, will clear the body of the incomming request. | ||
| clear_route_cache: If true, will enable clear_route_cache on the response. |
There was a problem hiding this comment.
why do we have clear_route_cache for responses? thats useless isnt it?
There was a problem hiding this comment.
If we do not clear the route cache on a request body no-op response (unless the envoy is configured to clear the route cache every time), A no-op request callout will cause the response callout to not be processed in every case. This is mainly for the request body as that is the one that is being processed before the response. It would be impractical to make one method for request bodies and one for response bodies.
There was a problem hiding this comment.
This is not the expected behavior.
First, clear_route_cache is only applicable to mainstream request path. Second, I don't follow the logic why clear_route_cache on request path could affect response path behavior
@jstraceski I think this should require more debugging
There was a problem hiding this comment.
So, the reason why the function says clear_route_cache on the response is because the returning datatype of this function is a BodyResponse, I have made a quick patch to make this more evident. This is the first pass I have done just on the callout_tools.py file, and some associated changes.
…ach method. Moving example specific code out of general tools class. Modifying normalization test to exercise all code paths from the example.
jstraceski
changed the title
jstraceski
changed the title
|
I checked the SDK build step locally, it appears that the command is malformed somehow, is it possible that the command line arguments are not being read correctly? |
|
bufbuild/buf#3000 looks like the buf folks may have some work to do. |
|
This should be good to go now with |
Thanks for the quick fix! |
fixing header -> body merge typo
Fixing add_body_mutation function.
Updated examples to reflect.
Updated tests to reflect.
Modified example to actually add a body rather than replacing.
Updated test to reflect.
Added test cases for body_clear.
Modifying wording of callout_tools.py methods to better reflect the intended purpose.
Moving some example specific code out of the general callout_tools.py file.
Modifying the normalization test to exercise all code paths from the example.
Modifying "request" to "callout" where applicable within callout_server.py.
Removing duplicated boilerplate class documentation from examples.
Cleaned up some examples, adding type hinting.
Phase 2
Cleaning up README.md and adding development buf resources.
Modifying requirement files to not overlap and to be clearer.
Refactoring / cleaning up requirements.
Clarifying and updating Docker instructions.
Reworking deny tool, adding common functions to parse headers and body.
Renaming address method to denote that it is for internal use.
Updating README.md to reflect address name change.
Cleaning up custom response example and test.
Updating basic test to reflect address naming change.
Removing duplicate/unused Dockerfiles.
Reworking jwt example for Dockerfile README.md section.
Reworking cloud_log example to use custom Dockerfile.
Adding default logging to all examples.
Updating README.md with a possible linting edgecase.