Support basic types by vinodreddy-g · Pull Request #46 · eclipse-score/persistency

vinodreddy-g · 2025-07-15T14:09:24Z

No description provided.

github-actions · 2025-07-15T14:13:01Z

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run //:license-check

Status: ⚠️ Needs Review

Click to expand output

[License Check Output]
Extracting Bazel installation...
Starting local Bazel server and connecting to it...
INFO: Invocation ID: 81b06c4c-ffa8-413c-aa9a-a9280fbde09b
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
DEBUG: Rule 'rules_boost~' indicated that a canonical reproducible form can be obtained by modifying arguments integrity = "sha256-aSViuHoOdF32Ue/Cx02/mOzyEebHMFuifjZs4GNWjsE="
DEBUG: Repository rules_boost~ instantiated at:
  <builtin>: in <toplevel>
Repository rule http_archive defined at:
  /home/runner/.bazel/external/bazel_tools/tools/build_defs/repo/http.bzl:387:31: in <toplevel>
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
WARNING: For repository 'googletest', the root module requires module version googletest@1.14.0, but got googletest@1.15.0 in the resolved dependency graph.
WARNING: For repository 'aspect_rules_lint', the root module requires module version aspect_rules_lint@1.0.3, but got aspect_rules_lint@1.4.2 in the resolved dependency graph.
Computing main repo mapping: 
Loading: 
Loading: 1 packages loaded
Analyzing: target //:license-check (2 packages loaded, 0 targets configured)
Analyzing: target //:license-check (2 packages loaded, 0 targets configured)

Analyzing: target //:license-check (58 packages loaded, 10 targets configured)

Analyzing: target //:license-check (106 packages loaded, 10 targets configured)

Analyzing: target //:license-check (156 packages loaded, 966 targets configured)

Analyzing: target //:license-check (165 packages loaded, 2141 targets configured)

Analyzing: target //:license-check (165 packages loaded, 2810 targets configured)

Analyzing: target //:license-check (168 packages loaded, 3776 targets configured)

Analyzing: target //:license-check (170 packages loaded, 5056 targets configured)

Analyzing: target //:license-check (170 packages loaded, 5060 targets configured)

Analyzing: target //:license-check (170 packages loaded, 5060 targets configured)

INFO: Analyzed target //:license-check (171 packages loaded, 6979 targets configured).
[9 / 13] JavaToolchainCompileClasses external/rules_java~/toolchains/platformclasspath_classes; 0s disk-cache
ERROR: /home/runner/work/inc_mw_per/inc_mw_per/BUILD:30:21: Generating Dash formatted dependency file ... failed: missing input file '//:cargo_lock'
ERROR: /home/runner/work/inc_mw_per/inc_mw_per/BUILD:30:21: Generating Dash formatted dependency file ... failed: 1 input file(s) do not exist
Target //:license.check.license_check failed to build
Use --verbose_failures to see the command lines of failed build steps.
ERROR: /home/runner/work/inc_mw_per/inc_mw_per/BUILD:30:21 Generating Dash formatted dependency file ... failed: 1 input file(s) do not exist
INFO: Elapsed time: 187.149s, Critical Path: 0.34s
INFO: 12 processes: 2 disk cache hit, 10 internal.
ERROR: Build did NOT complete successfully
ERROR: Build failed. Not running target

github-actions · 2025-07-15T14:14:48Z

The created documentation from the pull request is available at: docu-html

- Add basic types to kvsvalue - Add type info into the json format

- Added tests for all the basic types

- Rebase to main and take json-backend changes

- add eclipse license header

joshualicht · 2025-07-23T07:47:44Z

@vinodreddy-g Do you want to reduce the storage-space in the written json-files like mentioned in the FT?
So e.g. instead of

// Example of how KvsValue is stored in the JSON file (type-tagged format):
// {
//   "my_int": { "type": "I32", "value": 42 },
//   "my_float": { "type": "F64", "value": 3.1415 },
//   "my_bool": { "type": "Boolean", "value": true },
//   "my_string": { "type": "String", "value": "hello" },
//   "my_array": { "type": "Array", "value": [ ... ] },
//   "my_object": { "type": "Object", "value": { ... } },
//   "my_null": { "type": "Null", "value": null }
// }

Something like this maybe:

// Example of how KvsValue is stored in the JSON file (type-tagged format):
// {
//   "my_int": { "t": "i32", "v": 42 },
//   "my_float": { "t": "f64", "v": 3.1415 },
//   "my_bool": { "t": "bool", "v": true },
//   "my_string": { "t": "str", "v": "hello" },
//   "my_array": { "t": "arr", "v": [ ... ] },
//   "my_object": { "t": "obj", "v": { ... } },
//   "my_null": { "t": "null", "v": null }
// }

When you first see this you probably need to open a documentation to understand the t and v, but i think they are mostly self explaining.

Also additional info from @vbogdanb : Please use lowercase letters for the types (I32 -> i32)

arkjedrz · 2025-07-23T06:33:31Z

src/rust/rust_kvs/src/json_backend.rs

@@ -1,46 +1,197 @@
+// Copyright (c) 2025 Contributors to the Eclipse Foundation


Unrelated to this change: maybe missing license headers can be prevented with pre-commit hook, or at least CI task?

Ci task has to merged from central repo .I will ask dan.

src/rust/rust_kvs/tests/common.rs

src/rust/rust_kvs/src/kvs_value.rs

src/rust/rust_kvs/src/json_backend.rs

arkjedrz · 2025-07-23T09:43:33Z

src/rust/rust_kvs/src/json_backend.rs

+                            }
+                        }
+                        "Null" => {
+                            if let JsonValue::Null = value {


You can just return KvsValue::Null. But this raises a question - how can null be distinguished from erroneous condition?

Will fix the error handling and few things are missing as new PR.

src/rust/rust_kvs/src/json_backend.rs

- Chnaged the "type" to "t" and "value" to "v" - And used smaller case for type info in json

- Fixed the review comments on kvs_value

- Fixed the parsing in backend - Fixed the tests

src/rust/rust_kvs/tests/common.rs

arkjedrz · 2025-08-01T06:47:35Z

src/rust/rust_kvs/tests/common.rs

-        (_, _) => false,
+        _ => false,
    }
+    // ...existing code...


What is the meaning of this comment?

removed it.

arkjedrz · 2025-08-01T07:18:05Z

src/rust/rust_kvs/src/json_backend.rs

-                    .map(|(k, v)| (k.clone(), KvsValue::from(v)))
-                    .collect(),
-            ),
+            JsonValue::Number(_)


Remaining types can be handled with _ => KvsValue::Null.

arkjedrz · 2025-08-01T07:21:01Z

src/rust/rust_kvs/src/json_backend.rs

-            JsonValue::Boolean(b) => KvsValue::Boolean(b),
-            JsonValue::String(s) => KvsValue::String(s),
-            JsonValue::Null => KvsValue::Null,
+            JsonValue::Object(mut obj) => {


No mut would be preferred, but it's ok.

src/rust/rust_kvs/src/json_backend.rs

src/rust/rust_kvs/tests/kvs_new_wo_defaults_builder.rs

src/rust/rust_kvs/tests/kvs_default_values.rs

src/rust/rust_kvs/src/kvs_value.rs

- Taking ownership of JsonValue and KvsValue in backend file.

- changed the kvsvalue structure - changed the json_backend default matching arm

- Used index.get as before

- error message checks in kvs_value.rs

vinodreddy-g · 2025-08-01T11:59:45Z

@arkjedrz fixed the comments

…sic_types

vinodreddy-g requested review from PiotrKorkus and arkjedrz July 15, 2025 14:09

vinodreddy-g force-pushed the vinod_support_basic_types branch from 1afa640 to 4149339 Compare July 15, 2025 14:12

guysagnes requested a review from joshualicht July 15, 2025 14:12

vinodreddy-g mentioned this pull request Jul 15, 2025

Improvement: Support of the primitive types missing #42

Closed

vinodreddy-g marked this pull request as draft July 18, 2025 11:37

vinodreddy-g added 2 commits July 22, 2025 09:27

impl: add basic types

cbfcc28

- Add basic types to kvsvalue - Add type info into the json format

impl: add tests for types

64628b4

- Added tests for all the basic types

vinodreddy-g force-pushed the vinod_support_basic_types branch from 4149339 to 54e67a6 Compare July 22, 2025 08:51

vinodreddy-g marked this pull request as ready for review July 22, 2025 08:52

impl: take json_backend changes

2bdefbc

- Rebase to main and take json-backend changes

vinodreddy-g force-pushed the vinod_support_basic_types branch from 54e67a6 to 2bdefbc Compare July 22, 2025 08:53

impl: add license header

2142c45

- add eclipse license header

arkjedrz requested changes Jul 23, 2025

View reviewed changes

This was referenced Jul 25, 2025

CPP: Support basic types + Add Dir Parameter #54

Merged

Too early kvs init in kvs_tool #51

Closed

vinodreddy-g added 3 commits July 31, 2025 08:42

impl: change to aligned format in json file

7f24d66

- Chnaged the "type" to "t" and "value" to "v" - And used smaller case for type info in json

impl: fix review comments

9a09005

- Fixed the review comments on kvs_value

impl: fix the json format changes

072f9c9

- Fixed the parsing in backend - Fixed the tests

vinodreddy-g force-pushed the vinod_support_basic_types branch from bfd259b to 072f9c9 Compare July 31, 2025 12:14

vinodreddy-g requested a review from arkjedrz July 31, 2025 12:32

arkjedrz requested changes Aug 1, 2025

View reviewed changes

impl: Taking ownership of JsonValue in backend

d08963c

- Taking ownership of JsonValue and KvsValue in backend file.

vinodreddy-g force-pushed the vinod_support_basic_types branch from e26aba4 to d08963c Compare August 1, 2025 11:04

vinodreddy-g added 2 commits August 1, 2025 13:39

impl: fixed review comments

1f37f81

- changed the kvsvalue structure - changed the json_backend default matching arm

impl: reverted the changes for array get in tests

0979081

- Used index.get as before

vinodreddy-g requested a review from arkjedrz August 1, 2025 11:46

impl: Add error message checks

bd0d88d

- error message checks in kvs_value.rs

vinodreddy-g force-pushed the vinod_support_basic_types branch from b4bd6e7 to bd0d88d Compare August 1, 2025 11:56

arkjedrz approved these changes Aug 1, 2025

View reviewed changes

vinodreddy-g requested a review from qor-lb August 1, 2025 13:14

qor-lb approved these changes Aug 1, 2025

View reviewed changes

vinodreddy-g merged commit c3e0baf into eclipse-score:main Aug 1, 2025
11 checks passed

atarekra pushed a commit to Valeo-S-CORE-Organization/persistency that referenced this pull request Nov 9, 2025

Merge pull request eclipse-score#46 from qorix-group/vinod_support_ba…

be1684f

…sic_types

		@@ -1,46 +1,197 @@
		// Copyright (c) 2025 Contributors to the Eclipse Foundation

Conversation

vinodreddy-g commented Jul 15, 2025

Uh oh!

github-actions bot commented Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

License Check Results

Uh oh!

github-actions bot commented Jul 15, 2025

Uh oh!

joshualicht commented Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arkjedrz Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

vinodreddy-g Jul 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

arkjedrz Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

vinodreddy-g Jul 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

arkjedrz Aug 1, 2025

Choose a reason for hiding this comment

Uh oh!

vinodreddy-g Aug 1, 2025

Choose a reason for hiding this comment

Uh oh!

arkjedrz Aug 1, 2025

Choose a reason for hiding this comment

Uh oh!

vinodreddy-g Aug 1, 2025

Choose a reason for hiding this comment

Uh oh!

arkjedrz Aug 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vinodreddy-g commented Aug 1, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions bot commented Jul 15, 2025 •

edited

Loading

joshualicht commented Jul 23, 2025 •

edited

Loading

vinodreddy-g Jul 31, 2025 •

edited

Loading

vinodreddy-g Jul 31, 2025 •

edited

Loading