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

Fix Configuration Parser For Size Units #447

Open
3 tasks
LucioDonda opened this issue Dec 19, 2024 · 1 comment
Open
3 tasks

Fix Configuration Parser For Size Units #447

LucioDonda opened this issue Dec 19, 2024 · 1 comment
Assignees
Labels

Comments

@LucioDonda
Copy link
Member

LucioDonda commented Dec 19, 2024

Wazuh version Component Install type Install method Platform
5.0.0 Configration Parser Agent Sources -

Description

After the implementation #385 it was forced to implement a workaround on the configuration parser in order to correctly handle the size units.

LINK

        // This is a workaround for parsing size units
        if (std::string_view(key) == "batch_size")
        {
            should_parse_size = true;
        }

Initially it was implemented using size_t as the type but that was opening a possible conflict with using size_t as his own (e.g. while parsing threads).
Later another option was creating a custom type but that need to shared the header where it's defined in every module that uses it.
Another option was getting the output type and the parsing type from arguments of the template.

  • Design a solution that solves the previous issues.
  • Apply changes.
  • Correct and / or add any Unit Test needed.

It may also be considered, creating a new public interface to the configuration parser that returns values with a function AsType(), for example:

  • .GetConfig("table", "key").AsInt()
  • .GetConfig("table", "key").AsString()
  • .GetConfig("table", "key").AsTime()
  • .GetConfig("table", "key").AsSize()
@jr0me
Copy link
Member

jr0me commented Jan 31, 2025

Update

  • Looked into requirements
  • Created branch
  • Made ParseSizeUnit and ParseTimeUnit in ConfigurationParser public and static.
  • Removed the size and time special cases from ConfigurationParser::GetConfig

The idea is that GetConfig does a simply parsing of a configuration file and casts the values to the specified type. Any additional parsing and conversion must be done separately, a value like 1000MB is a string, and must by retrieved as a string, prior to be parsed as a size in bytes. It still needs to be decided how to improve the ConfigurationParser interface to make these conversions on the go, and to clamp values and set defaults, without pushing too much into the user, but GetConfig will be as simple as possible, maybe additional methods will be provided.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: On hold
Development

When branches are created from issues, their pull requests are automatically linked.

3 participants