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

default value calculation #2259

Closed
wants to merge 3 commits into from
Closed

default value calculation #2259

wants to merge 3 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Sep 30, 2018

After finishing Cassandra + LCDproc's configuration investigations, here are my findings until now.

I tried to filter between default value calculation and auto-detection. My criteria for which settings goes into which bucket was based on the need for system information. If it could be done alone via elektra (e.g. just information from another key) I added it to default value calculation.

I did not add it to the issue as the README is much more readable this way.

@ghost ghost mentioned this pull request Sep 30, 2018
@markus2330
Copy link
Contributor

Is there anything for me to do? If yes, please describe your goal, e.g., did you want to do a classification of different kinds of auto-detection and default value calculation?

@ghost
Copy link
Author

ghost commented Sep 30, 2018

Basically this README is a requirement collection which you requested (it was even your first message).

I have got a few questions though to start off the discussion:

  1. Is my differentiation senseful for you? It was not that obvious for me to draw a line between auto-detection and default-value-calculation (dvc from now on).
    As an example I think 1) under dvc could even be some sort of auto detection as it dynamically extracts a string out of another one.

  2. My first impression is that dvc should be able to fulfill the following properties:

  • conditionals
  • math expressions
  • copying of other key values

But 4) under dvc for example expects a float whereas some require int (also under 4) ) Is this very difficult to take care of from the C part? Or should the implementation also "force" the user to add a type metadata?

@markus2330
Copy link
Contributor

Basically this README is a requirement collection which you requested (it was even your first message).

It should be clear from the document itself what it is for.

discussion

We should only start discussions if someone get stuck. If you find clear answers there is no need for a discussion (unless someone disagrees).

But 4) under dvc for example expects a float whereas some require int (also under 4) ) Is this very difficult to take care of from the C part? Or should the implementation also "force" the user to add a type metadata?

I do not understand the questions.

@ghost
Copy link
Author

ghost commented Oct 2, 2018

Well it is not that clear to me if everything which I put into Auto-detection or default value calculation is correct but since nobody disagrees I simply assume it is correct.

I do not understand the questions.

Assume that you have a division in your default value calculation and a floating point number would be the result. (assume as an example 5.66). Now what exactly should be returned? Some settings require full numbers, eg. 5 for the example. Some expect float, eg. 5.66. Some would even require more post-comma values, eg. 5.6632432. I think this is very difficult to get that right and I think the best way would be to always return truncated integers?

And concerning 1) in dvc where you want to extract a small part out of the keyname:
I think this is too specific to be part in the default value calculation and should be done in another plugin. Do you agree with that?

@markus2330
Copy link
Contributor

Well it is not that clear to me if everything which I put into Auto-detection or default value calculation is correct but since nobody disagrees I simply assume it is correct.

Classifications are usually not correct, they might be useful nevertheless.

I think this is very difficult to get that right and I think the best way would be to always return truncated integers?

Of course serialization of floating points has some challenges. It is the purpose of the specification to describe what an application expects from its configuration. If there is an application for which some minimal precision is required, we should check that. If the application does not care there is no need to specify that.

I think this is too specific to be part in the default value calculation and should be done in another plugin. Do you agree with that?

Even the "default value calculation" does not have to be in one plugin. It is challenging to find the right balance of modularity and ease-of-use.

@ghost ghost changed the title default value calculation findings default value calculation Oct 13, 2018
@ghost ghost added the ready to merge label Oct 13, 2018
@@ -0,0 +1,131 @@
# Default value calculation

This Markdown file shortly describes my found requirements for default value calculation:
Copy link
Contributor

Choose a reason for hiding this comment

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

From where did you get any information. How did you come to the conclusions?

Copy link
Author

Choose a reason for hiding this comment

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

By looking through all of cassandras and lcdproc's configurations as you suggested.

This is more like a proposal like every other plugin PR we did.Especially the README.md is of interest. The Findings.md is just for backing up my suggested plugin solution and argue why I chose certain properties of the plugin.

Copy link
Contributor

Choose a reason for hiding this comment

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

You need to put contexts in the files themselves, otherwise it is not possible to understand or review your PRs (or other texts).

Copy link
Contributor

Choose a reason for hiding this comment

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

Btw. the problem is throughout the text, I only marked the first place. For example it is unclear if the later section "Auto-Detection" now also describes requirements and if it is derived from the same applications.

@ghost ghost closed this Mar 30, 2019
@ghost ghost deleted the default-value-calculation branch March 30, 2019 09:21
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants