Skip to content
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

Adds cmd option to just check config #256

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

TheRandomCharacter
Copy link
Collaborator

Instead of starting dataplane returns result of dataplane config read and check.

Comment on lines +268 to +269
eResult cDataPlane::DryRun(const std::string& configFilePath)
{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
eResult cDataPlane::DryRun(const std::string& configFilePath)
{
eResult cDataPlane::DryRun(std::string_view configFilePath)
{

We can use lightweight wrapper around const char* instead of creating dynamic std::string

Comment on lines +103 to 104
eResult DryRun(const std::string& configFilePath);

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

Comment on lines +270 to +271
return parseConfig(configFilePath);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that means this function should also accept std::string_view, which in turn implies that init function should accept std::string_view.

In parseConfig we should do this, instead of creating std::ifstream from std::string:

eResult cDataPlane::parseConfig(std::string_view configFilePath)
{
    eResult result = eResult::success;
    std::filesystem::path path(configFilePath);
    std::ifstream fromFileStream(path);

In summary, the following diff is all we need:

diff --git a/dataplane/dataplane.cpp b/dataplane/dataplane.cpp
index 97d9c20..fcf4577 100644
--- a/dataplane/dataplane.cpp
+++ b/dataplane/dataplane.cpp
@@ -74,7 +74,7 @@ cDataPlane::~cDataPlane()
 }
 
 eResult cDataPlane::init(const std::string& binaryPath,
-                         const std::string& configFilePath)
+                         std::string_view configFilePath)
 {
 	eResult result = eResult::success;
 
@@ -265,7 +265,7 @@ eResult cDataPlane::init(const std::string& binaryPath,
 	return result;
 }
 
-eResult cDataPlane::DryRun(const std::string& configFilePath)
+eResult cDataPlane::DryRun(std::string_view configFilePath)
 {
 	return parseConfig(configFilePath);
 }
@@ -1867,11 +1867,12 @@ void cDataPlane::switch_worker_base()
 	controlPlane->switchBase();
 }
 
-eResult cDataPlane::parseConfig(const std::string& configFilePath)
+eResult cDataPlane::parseConfig(std::string_view configFilePath)
 {
 	eResult result = eResult::success;
+	std::filesystem::path path(configFilePath);
+	std::ifstream fromFileStream(path);
 
-	std::ifstream fromFileStream(configFilePath);
 	if (!fromFileStream.is_open())
 	{
 		YADECAP_LOG_ERROR("can't open file '%s'\n", configFilePath.data());
diff --git a/dataplane/dataplane.h b/dataplane/dataplane.h
index f1437ee..fc737f6 100644
--- a/dataplane/dataplane.h
+++ b/dataplane/dataplane.h
@@ -99,8 +99,8 @@ public:
 	~cDataPlane();
 
 	eResult init(const std::string& binaryPath,
-	             const std::string& configFilePath);
-	eResult DryRun(const std::string& configFilePath);
+	             std::string_view configFilePath);
+	eResult DryRun(std::string_view configFilePath);
 
 	void start();
 	void join();
@@ -122,7 +122,7 @@ public:
 	std::string InterfaceNameFromPort(tPortId id) { return std::get<0>(ports[id]); };
 
 protected:
-	eResult parseConfig(const std::string& configFilePath);
+	eResult parseConfig(std::string_view configFilePath);
 	eResult parseJsonPorts(const nlohmann::json& json);
 	std::optional<std::map<tCoreId, CPlaneWorkerConfig>> parseControlPlaneWorkers(const nlohmann::json& config);
 	std::optional<std::pair<tCoreId, CPlaneWorkerConfig>> parseControlPlaneWorker(const nlohmann::json& cpwj);

@GeorgyKirichenko
Copy link
Collaborator

GeorgyKirichenko commented Nov 13, 2024

I think it would be better to use json-schema instead, possibly somewhere outside dataplane or controlplane

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.

3 participants