Add installation scripts for dependencies for node delta sharing. #972

RiteshKarki27 · 2025-11-03T14:28:01Z

No description provided.

Signed-off-by: riteshk <ritesh.karki@rwth-aachen.de>

Signed-off-by: riteshk <riteshkarki6@gmail.com>

…sharing Signed-off-by: Ritesh.K <riteshkarki6@gmail.com>

Signed-off-by: Ritesh.K <riteshkarki6@gmail.com>

…ring Signed-off-by: Ritesh.K <riteshkarki6@gmail.com>

al3xa23 · 2025-11-03T15:42:49Z

include/villas/nodes/delta_share/DeltaSharingRestClient.h

+#define __DELTASHARINGRESTCLIENT_H__
+
+#include <iostream>
+#include <nlohmann/json.hpp>


Why are you using this instead of jansson what we are alreay using?

The json library was changed in the last pull request. I think when I created a new one, it is also including all the previous commits made for this branch.
The json library was changed in this commit.

al3xa23 · 2025-11-03T15:46:56Z

include/villas/nodes/delta_share/Protocol.h

+
+        };
+
+        NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE(Format, provider)


If we stay with jansson, this would propably be more to be implemented like a json_t object?

stv0g · 2025-11-05T10:22:53Z

CMakeLists.txt

 cmake_dependent_option(WITH_NODE_WEBSOCKET  "Build with websocket node-type"                        "${WITH_DEFAULTS}" "WITH_WEB" OFF)
 cmake_dependent_option(WITH_NODE_ZEROMQ     "Build with zeromq node-type"                           "${WITH_DEFAULTS}" "LIBZMQ_FOUND; NOT WITHOUT_GPL" OFF)
 cmake_dependent_option(WITH_NODE_OPENDSS    "Build with opendss node-type"                          "${WITH_DEFAULTS}" "OpenDSSC_FOUND" OFF)
+cmake_dependent_option(WITH_NODE_DELTASHARING "Build with delta-sharing node-type"                      "${WITH_DEFAULTS}" "" ON)


Could you please fix the indentation, so that all options are aligned?

You are not checking the presence of any of the dependencies used by this node-type: arrow,...

stv0g · 2025-11-05T10:25:03Z

include/villas/nodes/delta_sharing/Protocol.h

@@ -0,0 +1,298 @@
+/* Node type: Delta Share.


What is the correct name of this protocol?

Delta Sharing

or Delta Share?

stv0g · 2025-11-05T10:25:44Z

include/villas/nodes/delta_sharing/Protocol.h

+#include <map>
+#include <string>
+#include <vector>
+#include <boost/optional.hpp>


We do not use Boost in this project. Please limit yourself to the C++20 standard library (which also provides an optional type).

stv0g · 2025-11-05T10:25:48Z

include/villas/nodes/delta_sharing/Protocol.h

+#include <boost/optional.hpp>
+#include <boost/optional/optional_io.hpp>
+
+namespace DeltaSharing


Please format this code with clang-format. There is also a script to do so in the tools folder, or you can use pre-commit.

stv0g · 2025-11-05T10:27:22Z

lib/nodes/delta_sharing/delta_sharing_client.cpp

+#include <thread>
+
+
+//namespace ds = arrow::dataset;


Please remove dead-code from the PR.

stv0g · 2025-11-05T10:33:39Z

include/villas/nodes/delta_sharing/functions.h

This is C++ code, please use the .hpp extension.

stv0g · 2025-11-05T10:33:45Z

include/villas/nodes/delta_sharing/delta_sharing_rest_client.h

This is C++ code, please use the .hpp extension.

stv0g · 2025-11-05T10:33:51Z

include/villas/nodes/delta_sharing/delta_sharing_client.h

This is C++ code, please use the .hpp extension.

stv0g · 2025-11-05T10:34:19Z

include/villas/nodes/delta_sharing/Protocol.h

This is C++ code, please use the .hpp extension.

Also, please use snake case capitalization for file names.

stv0g · 2025-11-05T10:35:26Z

lib/nodes/delta_sharing/delta_sharing_rest_client.cpp

Can we use curl as for the other web-based node-types? I dont think we should introduce another dependency for web-requests.

stv0g · 2025-11-05T16:05:19Z

Hi @RiteshKarki27,

I propose you directly push to the feature branch in this repo, rather than on your fork.

There seem to be some conflicts which stop me from merging this PR:

This would also unblock, from running the CI tests

RiteshKarki27 · 2025-11-07T07:07:46Z

The Delta Sharing Client I used for the implementing the Delta Sharing node is an unofficial client implementation mentioned in the GitHub page of the Delta Sharing Protocol.
The only changes I made was to replace the json library (nlohmann) with libjansson. If we were to use libcurl instead, then a lot of the client logic will have to be rebuilt.

This is also the reason why most of the header files have the .h extension since it came directly from their repo, but I will change those to .hpp and push the changes.

RiteshKarki27 · 2025-11-07T07:09:06Z

I will push my future commits directly to the feature branch and we can maybe keep this as a draft ticket until completely resolved.

stv0g · 2025-11-17T11:27:28Z

Hi @RiteshKarki27,

I thought again about your usage of nlohmann::json. And concluded, that in the long term, we should migrate VILLASnode to it. So, I think we can start using it already in the Delta Sharing node-type.

RiteshKarki27 · 2025-11-21T17:29:03Z

Hi @stv0g.
Great! I shall revert to the original implementation with nlohmann::json and push it to the feature branch.

JensenMax10 and others added 6 commits August 31, 2025 15:27

Add delta share node with deltaShare_read

24d6b90

Signed-off-by: riteshk <ritesh.karki@rwth-aachen.de>

Add implementation for deltaShare_write()

20ee955

Signed-off-by: riteshk <ritesh.karki@rwth-aachen.de>

Added integration test for delta_share node

2c382fe

Signed-off-by: riteshk <riteshkarki6@gmail.com>

Changed json library to use libjansson. Renamed delta_share to delta_…

92cb29e

…sharing Signed-off-by: Ritesh.K <riteshkarki6@gmail.com>

Improve code for node delta_sharing, rename files.

4f444dd

Signed-off-by: Ritesh.K <riteshkarki6@gmail.com>

Add installation scripts for Apache Arrow required for node delta-sha…

1b2adad

…ring Signed-off-by: Ritesh.K <riteshkarki6@gmail.com>

RiteshKarki27 requested review from n-eiling and stv0g as code owners November 3, 2025 14:28

al3xa23 reviewed Nov 3, 2025

View reviewed changes

stv0g reviewed Nov 5, 2025

View reviewed changes

al3xa23 deleted the branch VILLASframework:node-delta-sharing November 24, 2025 15:09

al3xa23 closed this Nov 24, 2025

		#include <thread>


		//namespace ds = arrow::dataset;

Add installation scripts for dependencies for node delta sharing. #972

Add installation scripts for dependencies for node delta sharing. #972

Uh oh!

Conversation

RiteshKarki27 commented Nov 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stv0g commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RiteshKarki27 commented Nov 7, 2025

Uh oh!

RiteshKarki27 commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stv0g commented Nov 17, 2025

Uh oh!

RiteshKarki27 commented Nov 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

stv0g commented Nov 5, 2025 •

edited

Loading

RiteshKarki27 commented Nov 7, 2025 •

edited

Loading