-
Notifications
You must be signed in to change notification settings - Fork 17
fix(deps): Updates to major ver of protovalidate #2284
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @dmihalcik-virtru, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
Hello team,
Gemini here, providing a summary of this pull request to help everyone get up to speed quickly.
This pull request, authored by dmihalcik-virtru, focuses on updating the project's dependencies. Specifically, it updates the protovalidate
library to a major new version (v0.12.0). According to the description, this update is necessary because the previous version was causing Dependabot to fail.
The changes involve updating the go.mod
and go.sum
files to reflect the new dependency version and its transitive dependencies. Additionally, the import paths for the protovalidate
library have been updated in several test files to match the new module path.
Highlights
- Dependency Update: The primary change is the update of the
protovalidate
dependency fromgithub.com/bufbuild/protovalidate-go v0.9.3
tobuf.build/go/protovalidate v0.12.0
. - Fix Dependabot Issue: This update is intended to resolve a failure encountered with Dependabot.
- Import Path Changes: The import path for the
protovalidate
library has been updated fromgithub.com/bufbuild/protovalidate-go
tobuf.build/go/protovalidate
in several test files. - Indirect Dependency Updates: Several indirect dependencies, including
cel.dev/expr
,github.com/google/cel-go
, and the generatedbufbuild/protovalidate
protobuf code, were also updated as a result of the main dependency change.
Changelog
Click here to see the changelog
- service/go.mod
- Removed direct dependency
github.com/bufbuild/protovalidate-go v0.9.3
(line 15). - Added direct dependency
buf.build/go/protovalidate v0.12.0
(line 8). - Updated indirect dependency
cel.dev/expr
fromv0.19.1
tov0.23.1
(line 56). - Added indirect dependency
github.com/bufbuild/protovalidate-go v0.9.1
(line 58). - Updated indirect dependency
buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go
to a newer timestamped version (line 63). - Updated indirect dependency
github.com/google/cel-go
fromv0.24.1
tov0.25.0
(line 113).
- Removed direct dependency
- service/go.sum
- Updated checksums corresponding to the dependency changes in
go.mod
(lines 1-6, 51-52, 162-163).
- Updated checksums corresponding to the dependency changes in
- service/policy/actions/actions_test.go
- Updated import path for
protovalidate
fromgithub.com/bufbuild/protovalidate-go
tobuf.build/go/protovalidate
(line 7).
- Updated import path for
- service/policy/attributes/attributes_test.go
- Updated import path for
protovalidate
fromgithub.com/bufbuild/protovalidate-go
tobuf.build/go/protovalidate
(line 8).
- Updated import path for
- service/policy/kasregistry/key_access_server_registry_test.go
- Updated import path for
protovalidate
fromgithub.com/bufbuild/protovalidate-go
tobuf.build/go/protovalidate
(line 7).
- Updated import path for
- service/policy/keymanagement/key_management_test.go
- Updated import path for
protovalidate
fromgithub.com/bufbuild/protovalidate-go
tobuf.build/go/protovalidate
(line 6).
- Updated import path for
- service/policy/namespaces/namespaces_test.go
- Updated import path for
protovalidate
fromgithub.com/bufbuild/protovalidate-go
tobuf.build/go/protovalidate
(line 6).
- Updated import path for
- service/policy/registeredresources/registered_resources_test.go
- Updated import path for
protovalidate
fromgithub.com/bufbuild/protovalidate-go
tobuf.build/go/protovalidate
(line 7).
- Updated import path for
- service/policy/resourcemapping/resource_mapping_test.go
- Updated import path for
protovalidate
fromgithub.com/bufbuild/protovalidate-go
tobuf.build/go/protovalidate
(line 6).
- Updated import path for
- service/policy/subjectmapping/subject_mapping_test.go
- Updated import path for
protovalidate
fromgithub.com/bufbuild/protovalidate-go
tobuf.build/go/protovalidate
(line 6).
- Updated import path for
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Dependencies shift and sway,
Major versions light the way.
Fixing bots with code so neat,
Making builds feel quite complete.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This PR updates protovalidate
to a major version (from github.com/bufbuild/protovalidate-go v0.9.3
to buf.build/go/protovalidate v0.12.0
) and adjusts import paths accordingly. The changes in go.mod
, go.sum
, and the test file import paths appear to correctly implement this dependency update.
The primary concern with a major version upgrade, especially for a validation library like protovalidate
, is the potential for breaking changes or subtle alterations in behavior that could impact existing functionality. The PR description includes a checklist for testing (unit tests, integration tests) which are currently unchecked.
It is crucial to ensure that comprehensive testing has been performed and that all tests pass with this new version. This includes verifying:
- Existing validations continue to work as expected.
- No new, unexpected validation errors are introduced.
- If
protovalidate
is used in performance-critical paths, that performance has not regressed.
Could you please update the PR description or comment on the status of the test suite with these changes? Specifically, confirming that all relevant tests pass or detailing any necessary updates made to tests due to legitimate changes in the new protovalidate
version.
Summary of Findings
- Testing Confirmation for Major Version Upgrade: The PR involves a major version upgrade of the
protovalidate
library. The checklist in the PR description for unit and integration tests is unchecked. It's critical to confirm that all tests pass with the new version to ensure no regressions or unexpected behavior changes have been introduced. This is a high-priority concern before merging.
Merge Readiness
The dependency update and import path changes in this PR are mechanically correct. However, due to the nature of a major version upgrade for a critical library like protovalidate
, and the currently unchecked testing checklist in the PR description, I recommend that this PR not be merged until it's confirmed that all relevant unit and integration tests pass with these changes. Addressing this testing confirmation is crucial to maintain code quality and prevent potential regressions. I am unable to approve the pull request myself; please ensure further review and approval by others once the testing status is clarified.
Benchmark results, click to expandBenchmark Results:
Bulk Benchmark Results
TDF3 Benchmark Results:
NANOTDF Benchmark Results:
Error Summary:
Standard Benchmark Metrics Skipped or Failed |
tests are covered with xtest and integration test
86fe853
to
1fdd850
Compare
Needs a re-review because I somehow decided to also fix the stray direct dep in the indirect block which is causing more merge conflicts |
Benchmark results, click to expandBenchmark Results:
Bulk Benchmark Results
TDF3 Benchmark Results:
NANOTDF Benchmark Results:
Error Summary:
Standard Benchmark Metrics Skipped or Failed |
This was causing dependabot to fail Updating this means we need a new connect too Also, resorts indirect and direct service go deps
1fdd850
to
7b77318
Compare
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Bulk Benchmark Results
TDF3 Benchmark Results:
NANOTDF Benchmark Results:
Error Summary:
Standard Benchmark Metrics Skipped or Failed |
### Proposed Changes * This was causing dependabot to fail ### Checklist - [x] I have added or updated unit tests - [x] I have added or updated integration tests (if appropriate) - [x] I have added or updated documentation ### Testing Instructions
🤖 I have created a release *beep* *boop* --- ## [0.5.5](service/v0.5.4...service/v0.5.5) (2025-05-30) ### Features * adds basic config root key manager ([#2303](#2303)) ([dd0d22f](dd0d22f)) * **policy:** cache SubjectConditionSet selectors in dedicated column maintained via trigger ([#2320](#2320)) ([215791f](215791f)) * **policy:** map and merge grants and keys ([#2324](#2324)) ([abf770f](abf770f)) ### Bug Fixes * **deps:** bump github.com/opentdf/platform/sdk from 0.4.5 to 0.4.7 in /service in the internal group ([#2334](#2334)) ([7f5a182](7f5a182)) * **deps:** Updates to major ver of protovalidate ([#2284](#2284)) ([39ad3c9](39ad3c9)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com>
Proposed Changes
Checklist
Testing Instructions