feature: use bytes as values in http proto #65

rustatian · 2025-01-13T12:21:15Z

Reason for This PR

ref: roadrunner-server/roadrunner#2104

Description of Changes

Use bytes instead of strings in http proto.

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the MIT license.

PR Checklist

[Author TODO: Meet these criteria.]
[Reviewer TODO: Verify that these criteria are met. Request changes if not]

All commits in this PR are signed (git commit -s).
The reason for this PR is clearly provided (issue no. or explanation).
The description of changes is clear and encompassing.
Any required documentation changes (code and docs) are included in this PR.
Any user-facing changes are mentioned in CHANGELOG.md.
All added/changed functionality is tested.

Summary by CodeRabbit

Dependencies
- Updated Go version to 1.23
- Updated multiple dependency versions, including Temporal API, gRPC, protobuf, and x/ packages
Protobuf Changes
- Modified HeaderValue message to use bytes instead of repeated string for value field
Subproject Updates
- Updated subproject commit reference in third-party API

Signed-off-by: Valery Piashchynski <piashchynski.valery@gmail.com>

coderabbitai · 2025-01-13T12:21:23Z

Walkthrough

This pull request introduces updates to the project's Go module configuration and protocol buffer definition. The changes primarily involve upgrading the Go version, toolchain, and various dependencies to their latest minor versions. Additionally, a modification in the HTTP header value type has been made in the protocol buffer definition, changing from a repeated string to bytes representation. A subproject commit identifier has also been updated.

Changes

File	Change Summary
`go.mod`	- Go version updated from 1.22 to 1.23 - Toolchain version updated from 1.22.5 to 1.23.4 - Multiple dependency versions incremented (Temporal API, gRPC, protobuf, etc.)
`proto/http/v1/http.proto`	`HeaderValue` message's `value` field type changed from `repeated string` to `bytes`
`third_party/api`	Subproject commit updated from `bae790a6cfe59936cb26e613af107780deea500e` to `5e65ea6afee8323fa7da18179c19aecc8f007f0b`

Poem

🐰 Hop, hop, through code so bright,
Dependencies dancing with delight!
Bytes and versions take their flight,
Upgrading systems with rabbit might,
Go version leaps to new height! 🚀

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e72c916 and eac5f2e.

⛔ Files ignored due to path filters (6)

build/temporal/api/enums/v1/reset.pb.go is excluded by !**/*.pb.go
build/temporal/api/errordetails/v1/message.pb.go is excluded by !**/*.pb.go
build/temporal/api/nexus/v1/message.pb.go is excluded by !**/*.pb.go
build/temporal/api/query/v1/message.pb.go is excluded by !**/*.pb.go
build/temporal/api/workflow/v1/message.pb.go is excluded by !**/*.pb.go
build/temporal/api/workflowservice/v1/request_response.pb.go is excluded by !**/*.pb.go

📒 Files selected for processing (1)

third_party/api (1 hunks)

✅ Files skipped from review due to trivial changes (1)

third_party/api

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a514b86 and e72c916.

⛔ Files ignored due to path filters (2)

build/http/v1/http.pb.go is excluded by !**/*.pb.go
go.sum is excluded by !**/*.sum

📒 Files selected for processing (2)

go.mod (1 hunks)
proto/http/v1/http.proto (1 hunks)

🔇 Additional comments (3)

go.mod (2)

3-5: Verify Go version compatibility

The upgrade to Go 1.23 and toolchain 1.23.4 is beyond my knowledge cutoff. Please ensure:

This version is officially released and stable

All dependencies are compatible with Go 1.23

No breaking changes affect the codebase

11-15: Verify dependency version compatibility

Multiple dependencies have been updated to versions from January 2025, which is beyond my knowledge cutoff. Please ensure:

These versions are stable releases

No breaking changes are introduced

Security vulnerabilities are addressed

proto/http/v1/http.proto (1)

Line range hint 11-21: Ensure comprehensive testing across all HeaderValue usages

The HeaderValue type is used in multiple messages:

Request (header, cookies, attributes)

Header (header)

Response (headers)

Please ensure:

Test cases cover all these different usages

Performance benchmarks compare old vs new implementation

Edge cases are handled (empty values, large payloads, special characters)

Also applies to: 22-24, 25-28, 29-34

coderabbitai · 2025-01-13T12:23:44Z

proto/http/v1/http.proto


 message HeaderValue {
-  repeated string value = 1;
+  bytes value = 1;


⚠️ Potential issue

Breaking Change: HeaderValue type modification requires careful migration

The change from repeated string to bytes for HeaderValue is a breaking change that will require updates in all clients using this API. While this aligns with the PR objective and potentially improves performance, please ensure:

Document the breaking change in CHANGELOG.md

Provide migration guidelines for clients

Consider backward compatibility strategy

Update all dependent repositories

This change could improve performance by reducing serialization overhead, but requires careful handling of encoding/decoding on both server and client sides.

Let's verify the impact:

#!/bin/bash # Find all usages of HeaderValue in the codebase rg -l "HeaderValue" --type go # Find potential protocol buffer imports that might be affected rg -l "proto.Marshal|proto.Unmarshal" --type go

Signed-off-by: Valery Piashchynski <piashchynski.valery@gmail.com>

feature: use bytes as values in http proto

e72c916

Signed-off-by: Valery Piashchynski <piashchynski.valery@gmail.com>

rustatian self-assigned this Jan 13, 2025

rustatian added C-enhancement Category: enhancement. Meaning improvements of current module, transport, etc.. C-future-compatibility Category: future compatibility labels Jan 13, 2025

coderabbitai bot reviewed Jan 13, 2025

View reviewed changes

chore: update temporal api to the latest version

eac5f2e

Signed-off-by: Valery Piashchynski <piashchynski.valery@gmail.com>

rustatian merged commit fe04b1e into master Jan 13, 2025
1 check passed

rustatian deleted the feature/use-bytes-as-values branch January 13, 2025 15:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feature: use bytes as values in http proto #65

feature: use bytes as values in http proto #65

Uh oh!

rustatian commented Jan 13, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Jan 13, 2025 •

edited

Loading

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot Jan 13, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

feature: use bytes as values in http proto #65

feature: use bytes as values in http proto #65

Uh oh!

Conversation

rustatian commented Jan 13, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reason for This PR

Description of Changes

License Acceptance

PR Checklist

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Jan 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Poem

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Jan 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rustatian commented Jan 13, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Jan 13, 2025 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)