Skip to content

Add string based backend#59

Merged
awegrzyn merged 3 commits intoAliceO2Group:masterfrom
ktf:string-backend
May 22, 2020
Merged

Add string based backend#59
awegrzyn merged 3 commits intoAliceO2Group:masterfrom
ktf:string-backend

Conversation

@ktf
Copy link
Member

@ktf ktf commented May 19, 2020

A URL format as:

str://key=value;key2=value2;key2.key3=value3...

is interpreted as a ptree.

@ktf
Copy link
Member Author

ktf commented May 19, 2020

@awegrzyn @vascobarroso with this DPL ConfigParamSpecs can be configured with (almost) the same syntax that ConfigurableParam uses for the command line, i.e.:

str://key=value;key2=value2;key2.key3=value3...

which is one of (my understanding of) the complains of @shahor02 for the DPL UI of the configuration mechanism. Ruben with this you can do:

--cfg str://key=value;key.key2=value2;key.key2.key3=value3

to set the values declared by ConfigParamSpec. I will add later some sugar to drop the need for str://. Notice that one needs to declare only "key" as ConfigParamSpec, if you use it as a ptree (or you could also the ROOT based mechanism showcased in the meeting).

A string formatter as:

str://key=value;key2=value2;key2.key3=value3...

is interpreted as a ptree.
@ktf ktf force-pushed the string-backend branch from 0dae9e1 to 1cb6837 Compare May 19, 2020 23:28
@awegrzyn
Copy link
Collaborator

@ktf LGTM, do you mind if I add some docs and unit test before merging?

@ktf
Copy link
Member Author

ktf commented May 20, 2020

@awegrzyn sure. Do you want me to do them?

@awegrzyn
Copy link
Collaborator

nah it's fine, I'll do it, I expect to do some refactoring anyway.

@awegrzyn awegrzyn self-assigned this May 20, 2020
@ktf
Copy link
Member Author

ktf commented May 20, 2020

I was actually thinking one cool thing to have would be to pass a LUA script to create the options in a programmatic way. ;-)

@awegrzyn
Copy link
Collaborator

Sure, but not sure I understand fully. Just lemme know how do you prefer to proceed.

@ktf
Copy link
Member Author

ktf commented May 20, 2020

That was an half joke and not for this PR... ;-)

@teo
Copy link
Member

teo commented May 20, 2020

@ktf Any reason for using ; as separator?
If it was , the input would be valid CSV, same as e.g. Ansible and AliECS parameter input.

@awegrzyn
Copy link
Collaborator

Good to go from my side although I'm also interested about ; vs ,

@ktf
Copy link
Member Author

ktf commented May 20, 2020

From my point of view, just compatibility with what is done elsewhere in O2. I agree "," would be better / more natural. @sawenzel @shahor02 might have a reason why it was not done that way for ConfigurationParam?

@sawenzel
Copy link

I think it it's because ConfigurableParam supports vectored assignment (like an initializer list) for which we use ',' to follow C++ notation.
Example: "Foo.3dpos={1,2,3};Bar.x=3."

@ktf
Copy link
Member Author

ktf commented May 20, 2020

@sawenzel ok, understood, albeit Foo.3dpos={1,2,3},Bar.x=3 or even better Foo.3dpos=[1,2,3],Bar.x=3 would work as well no? The advantage of the latter being not requiring escaping when passing it on the command line. Anyways, I suggest we merge this and we keep the discussion in mind for later.

@ktf ktf requested a review from awegrzyn May 20, 2020 19:26
@sawenzel
Copy link

anything works as long as you parse it correctly

@ktf
Copy link
Member Author

ktf commented May 20, 2020

maybe I misunderstood your comment, but my point is that bash would choke on Foo.3dpos={1,2,3};Bar.x=3 unless you quoted it or escaped the { } ;, because they have a special meaning. Anyways, I guess we agree that we should start with a compatible implementation and then evolve it, if needed / required.

@awegrzyn awegrzyn merged commit bdbb3b5 into AliceO2Group:master May 22, 2020
@ktf ktf deleted the string-backend branch May 22, 2020 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants