kratos icon indicating copy to clipboard operation
kratos copied to clipboard

[Feature] Replace `PGV` by `protovalidate`

Open perriea opened this issue 1 year ago • 6 comments

Feature-request

PGV has entered in maintenance and protovalidate have been announced in April 2023.

  • [ ] Migrate :
    • [x] this repository (https://github.com/go-kratos/kratos/pull/3084),
    • [ ] layout
    • [ ] examples,
  • [ ] Replace actual documentation
    • [ ] Give the protocol of migration : https://github.com/bufbuild/protovalidate/blob/main/docs/migrate.md
  • [ ] ...

perriea avatar Nov 12 '23 13:11 perriea

🤖

Hello @perriea,

Nice to meet you! I'm Dosu, a bot here to assist you with bugs, answer your questions, and guide you on your journey to becoming a contributor. While we wait for a human maintainer to join us, feel free to ask me anything about the Kratos repository. I'm here to help!

It seems like you've got this under control, if you want help or have specific questions, let me know what I can do for you!

dosubot[bot] avatar Nov 12 '23 13:11 dosubot[bot]

SGTM, can I entrust you with completing this task?

shenqidebaozi avatar Nov 13 '23 02:11 shenqidebaozi

can I entrust you with completing this task?

Yes, but, I need to explore how add protovalidate in the core and if it's complicated or not :

  • PGV generate Go/Java/C++/... code to verify inputs,
  • protovalidate is based on CEL (Common Expression Language), no code is generated and use his runtime : https://github.com/bufbuild/protovalidate/issues/47#issuecomment-1633213067

perriea avatar Nov 13 '23 09:11 perriea

Finaly, It's simple (after one call HTTP) :

2023-11-13T23:57:00.408+0100 [ERROR] app: args="filters:{values:\"1\"}  sort:{argument:Ticker}  pagination:{items:1  page:-1}" caller=recovery/recovery.go:59 code=400 component=http kind=server latency=0.001073208 operation=/instruments.v1.Instruments/Search reason=VALIDATOR
  stack=
  | error: code = 400 reason = VALIDATOR message = validation error:
  |  - pagination.page: value must be greater than or equal to 1 [int32.gte] metadata = map[] cause = validation error:
  |  - pagination.page: value must be greater than or equal to 1 [int32.gte]
  • On tests, obviously, it's a fail, I don't know how to repair because now, it's not a classic struct but a protoreflect.ProtoMessage,
  • protovalidate-go require google.golang.org/grpc v1.57.0, today google.golang.org/grpc v1.56.1 (shared dependencies between all packages),
  • I will do more tests tomorrow.

perriea avatar Nov 13 '23 23:11 perriea

Hi, @perriea,

I'm helping the Kratos team manage our backlog and am marking this issue as stale. From what I understand, you requested replacing PGV with protovalidate in the Kratos repository, layout, and examples, along with updating the documentation and providing a migration protocol. There were updates on your progress, including exploring the process of adding protovalidate in the core and encountering challenges with testing and dependencies.

Could you please confirm if this issue is still relevant to the latest version of the Kratos repository? If it is, please let the Kratos team know by commenting on the issue. Otherwise, feel free to close the issue yourself, or it will be automatically closed in 7 days.

Thank you for your understanding and cooperation.

I

dosubot[bot] avatar Feb 13 '24 16:02 dosubot[bot]

Hi, @perriea,

I'm helping the Kratos team manage our backlog and am marking this issue as stale. From what I understand, you requested replacing PGV with protovalidate in the Kratos repository, layout, and examples, along with updating the documentation and providing a migration protocol. There were updates on your progress, including exploring the process of adding protovalidate in the core and encountering challenges with testing and dependencies.

Could you please confirm if this issue is still relevant to the latest version of the Kratos repository? If it is, please let the Kratos team know by commenting on the issue. Otherwise, feel free to close the issue yourself, or it will be automatically closed in 7 days.

Thank you for your understanding and cooperation.

I

Pull request ready but not reviewed

perriea avatar Feb 13 '24 16:02 perriea

Hi, @perriea,

I'm helping the Kratos team manage their backlog and am marking this issue as stale. It looks like you opened this issue to request replacing PGV with protovalidate due to maintenance concerns and the announcement of protovalidate in April 2023. The migration involves updating the repository, layout, examples, and documentation, and providing a protocol for the migration process. There have been updates on the progress, including exploring the process of adding protovalidate in the core and encountering challenges with testing and dependencies. A pull request is ready but not yet reviewed.

Could you please let us know if this issue is still relevant to the latest version of the Kratos repository? If it is, please comment on the issue to let the Kratos team know. Otherwise, feel free to close the issue yourself, or it will be automatically closed in 7 days. Thank you!

dosubot[bot] avatar May 14 '24 16:05 dosubot[bot]