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

Enhance tmt --environment to also handle bool and numeric variables and values. #3279

Open
sbertramrh opened this issue Oct 10, 2024 · 8 comments

Comments

@sbertramrh
Copy link

Trying to pass a boolean, False, to try and disable pune:, did not end up disabling prune.

After collaborating with others on this we found the problem was the False ended up being passed in quotes to `prune:`` and so it did not disable it.

e,g,
So when manually set in the plan it passes :

11:21:47     DiscoverPlugin.delegate(step=discover, data=None, raw_data={'name': 'external_er', 'how': 'fmf', 'prune': False, 'filter': 'tag:-NoRHIVOS', 'url': 'https://gitlab.com/redhat/centos-stream/tests/kernel/kernel-tests.git', 'ref': 'main', 'test': ['/rt-tests/rt_ltp']})

But through the env variable using tmt

11:11:56     DiscoverPlugin.delegate(step=discover, data=None, raw_data={'name': 'external_er', 'how': 'fmf', 'prune': 'false', 'filter': 'tag:-NoRHIVOS', 'url': 'https://gitlab.com/redhat/centos-stream/tests/kernel/kernel-tests.git', 'ref': 'main', 'test': ['/rt-tests/rt_ltp']})

We have a workaround to make this case work but in general it would be nice to allow passing bool from the command line environment variable.
cc: @kkaarreell

@sbertramrh sbertramrh changed the title Enhance tmt --environment to also handle bool and numeric variables. Enhance tmt --environment to also handle bool and numeric variables and values. Oct 10, 2024
@kkaarreell
Copy link
Collaborator

Just realized, could you try False, not false?

@sbertramrh
Copy link
Author

Just realized, could you try False, not false?

We did both already. Problem is the quotes.

@happz
Copy link
Collaborator

happz commented Oct 10, 2024

Have you tried 0? See the paragraph about flag-like options at https://tmt.readthedocs.io/en/stable/overview.html#plugin-variables

@happz
Copy link
Collaborator

happz commented Oct 10, 2024

One more question, is it really —environment option, or environment variables set for the tmt itself? The former shouldn’t have an effect on plugin key.

@kkaarreell
Copy link
Collaborator

kkaarreell commented Oct 10, 2024

In the plan they have

prune: $VAR

and control the value through this variable.

@happz
Copy link
Collaborator

happz commented Oct 10, 2024

In the plan they have

prune: $VAR

and control the value through this variable.

Thank you, that explains what’s happening.

@sbertramrh
Copy link
Author

Because it will quote whatever is sent, 0 also ends up not disabling this one. Short answer, i did try 0 :) .
You are right it is not only when passed from the command line. We also updated the main.fmf and changed it to False but it still ended up activated because by the time it got to tmt it was passed as 'False'.

So in main.fmf

environment:
    PRUNE: True

And in the plan.fmf, as Karel mentioned above.
prune: ${PRUNE}

@happz
Copy link
Collaborator

happz commented Oct 10, 2024

Because it will quote whatever is sent, 0 also ends up not disabling this one. Short answer, i did try 0 :) . You are right it is not only when passed from the command line. We also updated the main.fmf and changed it to False but it still ended up activated because by the time it got to tmt it was passed as 'False'.

So in main.fmf

environment:
    PRUNE: True

And in the plan.fmf, as Karel mentioned above. prune: ${PRUNE}

Yes, I understand your problem now.

A couple of notes for myself: values of environment variables are internally handled as strings. After all, it's how they are delivered to tmt by OS, and FOO=1 is as string-ish as FOO=bar just as all command-line arguments start as strings even if they might look like numbers. There's no reason to treat the content of environment differently. The problem arises when the content of environment is used in other keys that are not necessarily expecting strings. The interpolation done by tmt at this stage is pretty dumb, it does not inspect the expected type of fields, basically just walks a big tree of dictionaries and lists, and if there's $... in a value, it's replaced with whatever is in the environment (and that's always a string). Need to check how step data is filled by these modified values, one would expect that normalization would perform some conversion of strings to booleans when assigning value to a boolean-annotated field.

Need to check when the validation happens, it should really complain about the field where the boolean is expected to be a string, any of them, plus need to check the normalization mixin whether it could apply some extra normalization of trivial data types. It all lets sneak a string to a place where a boolean is expected, which should never be allowed to happen.

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

No branches or pull requests

3 participants