-
Notifications
You must be signed in to change notification settings - Fork 2
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
Configuration design: Reflect n:m relation between stations and applications in the configuration #15
Comments
Proposal: Add groups to the mix. Groups are just sets of stations, but they can be hierarchically created. I don't think we need to preserve the hierarchy after initialization, but this is open to discussion.
During parsing, the information if a group is a predefined one based on a station name or profile name can be kept. This way it makes it easy to keep only the user defined groups and the profile based groups for filtering in the ui. Now you could define applications and the groups of stations it may run on.
This way, it is easy to support applications that run on different sets of stations and stations that supports many different applications without being to verbose. Note that my post may not contain perfectly valid YAML ... |
Definition: groups are sets of stations. Proposed syntax for group definition in EBNF:
Reserved words: We could also ignore whitespace, .e.g Example:
is equivalent to
|
Comments:
It would make sense to ask for something such as (WINDOWS and LEAP and GOOD_GPU), plus since term can be a group we are going to have to calculate it anyway.
So the way to implement this, I think, would be to read all the group declarations into a map and resolve them recursively (via dynamic programming = https://en.wikipedia.org/wiki/Dynamic_programming) using a flag to avoid cycles / infinite recursion. |
Absolutely. Fixed grammar:
Regarding dynamic programming: I think, simple recursion and a flag to indicate that a node in the recursion tree has already been visited but has not been resolved should be enough. Normally, dynamic programming is used to reduce computation time by reusing values that are computed in different recursion trees. But in our case, once the set of station for a group A is known, the string representation of group A is replaced by this set (i.e. the node becomes terminal). Hence, when the set of stations is computed for another group B, that uses group A in its definition, the set for group A is already known. If the same group is used twice in the call stack of a recursive resolution, we consider it an error. |
I am not sure about how to understand "not": e.g. what Also i do not like absence of underscores in IDs... is that intentional? Somehow i assumed IDs to be just valid variable identifiers... |
Allow
|
Regarding |
I would say that
and
... so "not c" is equivalent to -. It is true that
and
but that's the kind of problem that include/exclude schemes in commands such as rsync have. Likewise
|
Definitions
true
That's not true. We defined
I would also say, that In my definition, your example would yield
Assume your definition of of
What do you mean by
I'd rather say
i.e.
Again,
i.e. it's consistent with the above definition. Also, in your definition
which shouldn't happen.
PS.: Please use the quote feature ( PPS.: This post was written in a hurry. I hope I made no mistake in my (simple) math. |
Thank You Christian! Frankly it was also my understanding from our discussion that Now i can see that they are mostly equivalent: @porst17 Note that adding a new station changes the total set (all_stations) and thus changes all standalone complements, which means that many group definitions will have to be manually checked/updated by user after adding new stations if we choose to allow standalone set complements. I propose to forbid terms like |
@porst17 : I don't disagree with your math, I also agree your interpretation is reasonable and perhaps more reasonable when you consider "or" instead of "," (although I know and never disagreed with them being equivalente). We were simply thinking of different semantics. ... but I disagree with the usefulness of "not" if it's meant to be "complement". ... because then
And any use of "not" basically adds "all stations"... so it's only useful for lists such as:
which we shouldn't support, because you should NEVER indicate your app is compatible with "all stations except some" because you don't know which stations might be added in the future and can't be sure they'll be compatible. So it's dangerous and bad practice. So we can simply disallow "not" except in conjunctions
or change the syntax or whatever is needed so that individual stations and groups can be excluded from an enumeration (i.e. conjunction). More like the original intention of being able to write things such as: TOUCH, MULTITOUCH, !LEAP, !TEST without having to do something like TOUCH and not LEAP, TOUCH and not TEST, MULTITOUCH and not LEAP, MULTITOUCH and not TEST or writing more than one group definition. |
PS: If the app compatibility is: KIOSK and WIDESCREEN, STATIC, PROJECTOR then:
An alternative if you want pure set operations instead of enumeration semantics is to do like rsync and change the compatibleStations to two keys: compatibleStations so the app can be run on any station that is in the compatible list and not in the incompatible list. This, I think, allows us to remove the "not". |
My point was to show that our current definition has certain problems. If we rely on actual set operations, everything is clearly defined as long as we choose operator names wisely to avoid confusion. I am not saying that this is the route to go. But @elondaits' examples revealed some of the ambiguities and inconsistencies of the current approach. And like @elondaits said many times: If we don't understand it, the user will neither. |
To be constructive again: What about defining groups like in my very first post?
This will be the most likely use case in my opinion. And then allow another syntax:
In each
would be valid. The YAML parser takes care of preserving the hierarchy and distinguishes between I think, it makes sense to define that This feels rather intuitive to me and is very similar to what people (sys admins and devs) are used to. In contrast, the usual set theory just feels weird in the context of configuration files. |
BTW: |
What about the following YAML:
which means that we can interpret it and turn into |
In my sample above:
Update: one can also emulate intersection due to the usual trick Oh, looks like i came to something like reverse polish notation and tree representation again :-/ |
@malex984 The reason why I used If you really want to support intersection explicitly, then I would prefer a human readable keyword like What you posted is also not reverse polish notation. Because in Also, your tree is not obvious, because you actually have a list you interpret as a tree using certain additional knowledge. In my Next, your notation doesn't look complete or consistent to me: What does In my opinion, your new notation has the same problems our previous |
In order to come to an end, I would say we stick to
The tree is explicit, you don't need to parse strings except for comparing against the three keywords. You do not even need to trim whitespace. The YAML parser will do this for you. Determining the content of each groups if a trivial recursive algorithm that also checks for cycles in the definition. Of course, there are different ways in YAML to write down |
@porst17 I only tried to extend #15 (comment) and provide shortcuts for all operations. Evolution was something like: Sorry for confusion! I actually meant the usual polish notation with 3 operations as follows:
Note:
|
makes no sense to me. It looks like
the evaluation of the groups @malex984 @elondaits Are there any solid arguments against my proposal in #15 (comment)? If not, please close this issue and move forward. |
I hadn't commented on the proposal because I wanted to dedicate some time to thing it through. But my thoughts so far:
or
|
@porst17 I see, yes my shortcut for intersections seems misleading :-/ I agree with your proposal #15 (comment), it just seemed to me that your proposed syntax is a bit heavy. I hope your shortcut notation may be used everywhere as @elondaits elaborated at the end of #15 (comment). @porst17 What about the recursive usage? Consider for example:
|
@elondaits Intersection is useful because it reflects your idea of capabilities. I know, intersection can be emulated by the other operations, but this is too tedious. I would like to leave it in. We could argue about giving it a better name, but if no one of us comes up with a better name until tomorrow (that also received two up-votes by the other team members), we keep @malex984 The proposed notation does everything we need, is kind of self-explaining except for the order of evaluation (
with parentheses (i.e. non regular grammar) and unicode symbols Let's wait for the replacement proposals for |
I propose to keep |
I don't like intersectWith because it suddenly makes the user start thinking of set operations and it's not too clear how it works... I had to read the semantics to understand what it does and how it interacts with the other two. From above:
so if I want to limit the program to test windows stations that have a Leap and Sound: WINDOWS ∩ TEST ∩ LEAP ∩ SOUND
but it gets a bit out of hand and is not very readable. And also, I can't help thinking some people will expect this to be equivalent (and then wonder why it doesn't work):
because I though this might be a possibility before reading the semantics. So a more complex idea that makes the semantics clearer and the syntax more friendly is:
so the case above would simply be:
BUT since this is taking too much time and since intersectWith is just a way to write things shorter and not a change in the final result (which is just a set of stations in the end) I'm giving a thumbs up to the previous proposal so you're free to move ahead and we can check if this idea has any problems at the semantic level, since it's simple to implement but harder to think about). |
Wow! Good catch @elondaits! It seems to be exactly like with my intersection shortcut noted by @porst17 in #15 (comment) @elondaits Sorry, i do not see how I would say ps: that limitation could may solve the problem with my intersection shortcut as one can repeat items (for intersection) within a list: |
Thanks, @elondaits, for your comments. While reading it, I finally understood why our current approaches are all misleading in one way or the other: We are abusing YAML sequences ( I though about it and looked at the YAML specification again (actually, examples). It turned out that YAML has a data type for sets and it is obvious and I don't know why we missed it so far: The key sets of maps! Very long:
Still long:
Shorter (this is the official notation for sets):
Very short (and best for our purpose):
The YAML parser will turn each of the above into this canonical YAML:
Set properties (in YAML and in mathematics):
These things are not true for YAML sequences By switching from
would be
Inline definition of groups would stay mostly the same:
would all be valid to define
The only drawback of the I highly recommend to implement this (hopefully last) change, i.e. using the right ( Side note: Reserved words ( |
@porst17 Did i get it right that the shortcut for union is a set notation But using sequence of arguments for Side note: if |
I'm OK with using the set notation, but not with this:
... not because the concept is wrong, but because if you write [ instead of { (or viceversa) your set definition will not be right and you'll never understand why. It's a very small difference, visually. Here I'm just thinking of the "usability" / "friendliness" of the notation. Also my proposal used two different keywords (ifIn and ifInAll, which would be like "intersectWith" and "intersectWithEach") on purpose, because some modern languages / authors recommend not overloading a method for basically different types (in this case, a set and a collection of sets). This is not a method but in a way it's like a function / constructor. Once again, this is a design thing to improve readability / comprehension, not a problem with the underlying mechanism. ... Aaaalso... in the "intersectWith: [a,b]" variant we would be using a list of sets to differentiate the cases, but the order of the sets is not really important so conceptually it would take a set of sets, not a list/array of sets. So here [] is used for syntactic convenience, unlike {} which was proposed for conceptual reasons. |
I propose to
Possible extension may go as follows:
For example: Corresponding YAML dump (note difference in the order of items):
Note that above will not let one to define an intersection with a set, like
|
BTW, my proposal above is just a shortcut for |
elondaits To me using the set shortcut Therefore we better choose only one way: either some extended shortcut notation OR always ask for mappings: IMO: user will need to read some manual for defining station groups anyway in any case! |
People who write config files for programs should be able to distinguish
It's a little longer, but the parser would be able to indicate
Solved by requiring sequence-type arguments for
The conceptional problem is with recursive set definitions. With
we interpret |
@malex984 I find |
@Malex @elondaits I am deciding it now:
Each of the set/sequence elements is optional, i.e. The config file format requires a version tag (see #17 (comment)). If it really turns out that the above way of defining the groups is messy, cumbersome, confusing, inconsistent or whatever and we cannot fix it in a backwards-compatible manner, we can do config v2 and provide a converter from v1 to v2. Writing the converter probably requires less time than this discussion. 😉 |
I think this issue should be closed with the commit of some documentation or commented sample config. Having the spec as issues is messy and if it changes or we find a problem when we start implementing we end up programming against several issues. |
@elondaits Valid point. |
Current state: only initial parsing of
|
Note: missing functionality will be implemented within #14 |
…definition Now Groups can be defined completely following the design from #15 * Added more tests of groups (miniGroupHilbert*)
Fixes for Hilbert tool and generate_ogl.sh * Fixed handling Group definition due to #15 + more unit tests * Hilbert CLI server tool: better output & checking. Dry-Run. Improved control over client-side driver. * Extended Hilbert Client-side tool: services can be managed separately
Having a list of applications for each station/profile is not enough and having a list of stations/profiles for each application is not enough either. Having both seems to be a bad design decision.
Note: this is a part of super-issue "Configuration Design": #13
The text was updated successfully, but these errors were encountered: