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

feat: Add REST Interceptors to support reading metadata #2299

Merged
merged 8 commits into from
Dec 18, 2024

Conversation

parthea
Copy link
Contributor

@parthea parthea commented Dec 17, 2024

Fixes #2263 🦕

@product-auto-label product-auto-label bot added the size: l Pull request size is large. label Dec 17, 2024
@parthea parthea force-pushed the add-rest-response-metadata branch 2 times, most recently from 46d8633 to 19dbcc9 Compare December 17, 2024 15:10
@product-auto-label product-auto-label bot added size: xl Pull request size is extra large. and removed size: l Pull request size is large. labels Dec 17, 2024
@parthea parthea marked this pull request as ready for review December 17, 2024 15:14
@parthea parthea requested a review from a team as a code owner December 17, 2024 15:14
@parthea parthea requested a review from vchudnov-g December 17, 2024 15:14
@parthea parthea assigned vchudnov-g and unassigned ohmayr Dec 17, 2024
@parthea parthea added the owlbot:run Add this label to trigger the Owlbot post processor. label Dec 17, 2024
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Dec 17, 2024
@parthea parthea force-pushed the add-rest-response-metadata branch 2 times, most recently from e4ff750 to 587f99c Compare December 17, 2024 15:36
@product-auto-label product-auto-label bot added size: l Pull request size is large. and removed size: xl Pull request size is extra large. labels Dec 17, 2024
@product-auto-label product-auto-label bot added size: xl Pull request size is extra large. and removed size: l Pull request size is large. labels Dec 17, 2024
Copy link
Contributor

@vchudnov-g vchudnov-g left a comment

Choose a reason for hiding this comment

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

Consider my wording tweaks before merging.

after it is returned by the {{ service.name }} server but before
it is returned to user code.
it is returned to user code. This `post_{{ method.name|snake_case }}` iterceptor runs
before the `post_{{ method.name|snake_case }}_with_metadata` iterceptor.
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest

        DEPRECATED. Please use the `post_{{ method.name|snake_case }}_with_metadata`
        interceptor instead. 

        Override in a subclass to read or manipulate the response
        after it is returned by the {{ service.name }} server but before
        it is returned to user code. This `post_{{ method.name|snake_case }}` interceptor runs
        before the `post_{{ method.name|snake_case }}_with_metadata` interceptor.

(reworded slightly and fixed spelling of "interceptor")

Copy link
Contributor Author

@parthea parthea Dec 18, 2024

Choose a reason for hiding this comment

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

Fixed in 83cfa50. Good catch on the spelling error! I filed #2301 to see if we can add a presubmit to help with future development.

Comment on lines 401 to 407
Override in a subclass to read or manipulate the response or metadata after it
is returned by the {{ service.name }} server but before it is returned to user code.
This `post_{{ method.name|snake_case }}_with_metadata` interceptor runs after the
`post_{{ method.name|snake_case }}` iterceptor. If the response is modified in
`post_{{ method.name|snake_case }}`, the modified response will be used in
`post_{{ method.name|snake_case }}_with_metadata`. This `post_{{ method.name|snake_case }}_with_metadata`
interceptor is recommended for new development instead of `post_{{ method.name|snake_case }}`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Override in a subclass to read or manipulate the response or metadata after it
is returned by the {{ service.name }} server but before it is returned to user code.
This `post_{{ method.name|snake_case }}_with_metadata` interceptor runs after the
`post_{{ method.name|snake_case }}` iterceptor. If the response is modified in
`post_{{ method.name|snake_case }}`, the modified response will be used in
`post_{{ method.name|snake_case }}_with_metadata`. This `post_{{ method.name|snake_case }}_with_metadata`
interceptor is recommended for new development instead of `post_{{ method.name|snake_case }}`.
Override in a subclass to read or manipulate the response or metadata after it
is returned by the {{ service.name }} server but before it is returned to user code.
We recommend only using this `post_{{ method.name|snake_case }}_with_metadata`
interceptor in new development instead of the `post_{{ method.name|snake_case }}` interceptor.
When both interceptors are used, this `post_{{ method.name|snake_case }}_with_metadata` interceptor runs after the
`post_{{ method.name|snake_case }}` interceptor. The (possibly modified) response returned by
`post_{{ method.name|snake_case }}` will be passed to
`post_{{ method.name|snake_case }}_with_metadata`.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 11b803f

@parthea parthea merged commit e050f4e into main Dec 18, 2024
125 checks passed
@parthea parthea deleted the add-rest-response-metadata branch December 18, 2024 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: xl Pull request size is extra large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add REST Interceptors to support reading metadata
3 participants