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.

Sign up for GitHub

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

Jump to bottom

Streaming Interceptors #3641

Merged
merged 28 commits into from
Feb 9, 2025
Merged

Streaming Interceptors #3641

merged 28 commits into from
Feb 9, 2025

Conversation

douglaswth
Copy link
Member

@douglaswth douglaswth commented Feb 1, 2025 •
edited
Loading

  • Initial implementation of Streaming Interceptors generation based on Interceptors Proposal #3614
  • Add SendWithContext and RecvWithContext methods to the stream interfaces
  • Fix several generation bugs

@douglaswth
Add read/write streaming payload/result methods to interceptor DSL
9f853b8
@douglaswth
Weave streaming interceptor stuff through codegen
e547e64
@douglaswth
Do not try to use a field from an interface for the raw payload when …
d6d779d 
…wrapping streaming methods
@douglaswth
Fix golden files
fa0bdde
@douglaswth
Collect attributes from a method that actually has the interceptor ap…
946b677 
…plied; import the correct user types packages when rendering service_interceptors.go; fix an ancient typo
@douglaswth
Fix StreamingResult accessor signature and return
28789b0
@douglaswth
Add SendWithContext and RecvWithContext methods to the stream interfa…
905c5d8 
…ce generation
@douglaswth
Progress on wrapping streams with interceptors
5905958
@douglaswth
Finish generating the stream wrapping for interceptors; remove the En…
6e8e746 
…dpoint field from goa.InterceptorInfo struct since it is redundant with the next goa.Endpoint parameter sent to interceptors
@douglaswth
Progress on adding comments and fixing tests; fix a bug where refs we…
3826f70 
…re defs
@douglaswth
Finish fixing tests
0261a0c
@douglaswth
Add more tests for streaming interceptor service codegen
9c35929
@douglaswth
Fix bug where the CLI ParseEndpoint method would try to wrap every me…
cc0e16a 
…thod with interceptors even if they do not apply
raphael
raphael reviewed Feb 2, 2025
View reviewed changes
Copy link
Member

@raphael raphael left a comment

Choose a reason for hiding this comment

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

Really great PR! I left very few minor comments. Overall it all makes sense and follows the existing patterns very well, good job!

@douglaswth douglaswth requested a review from raphael February 3, 2025 20:11
douglaswth and others added 9 commits February 3, 2025 17:54
@douglaswth
Add a ReturnContext field to goa.InterceptorInfo to allow interceptor…
712ab19 
…s to modify the context returned by SendWithContext/RecvWithContext even after calling next
@douglaswth
Scrap ReturnContext field in favor of changing the interceptor interf…
3161e0b 
…ace to return a context itself
@douglaswth
Merge branch 'v3' into streaming-interceptors
c236fce
@douglaswth
Update Golden files to make the Interceptor tests happy again
6a734fe
@douglaswth
Change goa.InterceptorInfo to an interface that generated interceptor…
c36dfb6 
… info structs implement
@douglaswth
Separate StreamingPayload and StreamingResult methods to Client and S…
8035d69 
…erver variations
@douglaswth
Use goa.Endpoint for next again and do not return contexts from SendW…
2ba73ae 
…ithContext/ReceiveWithContext
@douglaswth
Merge branch 'v3' into streaming-interceptors
a1f2a91
@raphael
Address lint issues
edbbca8
@raphael raphael force-pushed the streaming-interceptors branch from cf121b1 to edbbca8 Compare February 8, 2025 21:31
@raphael raphael merged commit d581e4e into v3 Feb 9, 2025
11 checks passed
@raphael raphael deleted the streaming-interceptors branch February 9, 2025 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raphael raphael Awaiting requested review from raphael

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@douglaswth @raphael