Skip to content

Add client side schema validation#400

Closed
halfcrazy wants to merge 1 commit into
ovn-kubernetes:mainfrom
halfcrazy:client-validate
Closed

Add client side schema validation#400
halfcrazy wants to merge 1 commit into
ovn-kubernetes:mainfrom
halfcrazy:client-validate

Conversation

@halfcrazy

Copy link
Copy Markdown
Contributor

Added new validation routines to ensure client operations conform to the OVSDB schema before being sent to the server.

fix #160

@halfcrazy

halfcrazy commented May 13, 2025

Copy link
Copy Markdown
Contributor Author

GitHub Actions failed too. Need #399 to be merged to fix checks. It is recommended to add a dependent bot to avoid security issues and pipeline outdated.

@halfcrazy halfcrazy force-pushed the client-validate branch 2 times, most recently from 102007b to ea319b9 Compare May 16, 2025 10:39
@halfcrazy halfcrazy force-pushed the client-validate branch 3 times, most recently from 55228b4 to 237f980 Compare May 17, 2025 09:53
@halfcrazy halfcrazy changed the title add client side schema validation Add client side schema validation May 18, 2025
Signed-off-by: Yan Zhu <hackzhuyan@gmail.com>
@halfcrazy

Copy link
Copy Markdown
Contributor Author

ping @dave-tucker

@dave-tucker

Copy link
Copy Markdown
Collaborator

@halfcrazy sorry for the delay in reviewing this (and letting libovsdb CI get in the state it was).
I'll review the code properly later this week.
Would you mind adding a benchmark for api.Create as a seperate commit (ideally before these changes).
It would be good to understand how much overhead validation is adding.

@halfcrazy

Copy link
Copy Markdown
Contributor Author

@halfcrazy sorry for the delay in reviewing this (and letting libovsdb CI get in the state it was). I'll review the code properly later this week. Would you mind adding a benchmark for api.Create as a seperate commit (ideally before these changes). It would be good to understand how much overhead validation is adding.

Add benchmark test in #414

My local benchstat

goos: linux
goarch: amd64
pkg: github.com/ovn-kubernetes/libovsdb/client
cpu: Intel(R) Core(TM) i5-9600K CPU @ 3.70GHz
                          │    bench1    │                bench2                │
                          │    sec/op    │    sec/op     vs base                │
APICreate/simple_model-4    16.58µ ± ∞ ¹   16.75µ ± ∞ ¹       ~ (p=1.000 n=3) ²
APICreate/complex_model-4   32.32µ ± ∞ ¹   37.47µ ± ∞ ¹       ~ (p=0.100 n=3) ²
geomean                     23.15µ         25.05µ        +8.24%
¹ need >= 6 samples for confidence interval at level 0.95
² need >= 4 samples to detect a difference at alpha level 0.05

                          │    bench1     │                bench2                 │
                          │     B/op      │     B/op       vs base                │
APICreate/simple_model-4    3.690Ki ± ∞ ¹   3.880Ki ± ∞ ¹       ~ (p=0.100 n=3) ²
APICreate/complex_model-4   9.208Ki ± ∞ ¹   9.774Ki ± ∞ ¹       ~ (p=0.100 n=3) ²
geomean                     5.829Ki         6.158Ki        +5.64%
¹ need >= 6 samples for confidence interval at level 0.95
² need >= 4 samples to detect a difference at alpha level 0.05

                          │   bench1    │                bench2                │
                          │  allocs/op  │  allocs/op   vs base                 │
APICreate/simple_model-4    35.00 ± ∞ ¹   38.00 ± ∞ ¹        ~ (p=0.100 n=3) ²
APICreate/complex_model-4   143.0 ± ∞ ¹   169.0 ± ∞ ¹        ~ (p=0.100 n=3) ²
geomean                     70.75         80.14        +13.27%
¹ need >= 6 samples for confidence interval at level 0.95
² need >= 4 samples to detect a difference at alpha level 0.05

@dave-tucker

Copy link
Copy Markdown
Collaborator

@halfcrazy github decided to close this since I merged #414 🤦
please could you re-open 😆

@halfcrazy

Copy link
Copy Markdown
Contributor Author

@halfcrazy github decided to close this since I merged #414 🤦 please could you re-open 😆

OK, i will open a new pr.

@halfcrazy

Copy link
Copy Markdown
Contributor Author

#415

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.

Using schema to do a client-side row validate

2 participants