Skip to content

fix: save and restore pb state#9606

Merged
juststillthinking merged 4 commits into
apache:masterfrom
kingluo:fix-pb-state
Jun 7, 2023
Merged

fix: save and restore pb state#9606
juststillthinking merged 4 commits into
apache:masterfrom
kingluo:fix-pb-state

Conversation

@kingluo

@kingluo kingluo commented Jun 6, 2023

Copy link
Copy Markdown
Contributor

Description

Fixes #9270

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

leslie-tsang
leslie-tsang previously approved these changes Jun 7, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's a opentelemetry or grpc-transcode problem, add the test cases to opentelemetry2.t or grpc-transcode3.t, no need to create a new test file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, it doesn't belong to any plugin, this is a common pb status issue with any plugin. It's best to use a stand-alone test file to highlight this issue.

Comment on lines +102 to +104
local pb_old_state = pb.state(proto.pb_state)
local ret = handle_error_response(status_detail_type)
pb.state(pb_old_state)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why not do this in handle_error_response function, around pd.decode like other place.

Comment on lines +26 to +29
- example-plugin
- key-auth
- opentelemetry
plugin_attr:

@juststillthinking juststillthinking Jun 7, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't you need the grpc-transcode plugin to reproduce the problem?

@kingluo kingluo Jun 7, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No.

local compile_proto = require("apisix.plugins.grpc-transcode.proto").compile_proto

Mark my last words. It does not depend on a specific plugin. Instead, it's a general problem with lua-protobuf lib usage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

help request: opentelemetry + grpc-transcode not working

3 participants