Fixes #29254 - add display options to host creation on ovirt #507
Conversation
| } | ||
| end | ||
| compute_attributes.delete('display_type') | ||
| compute_attributes.delete('keyboard_layout') |
There was a problem hiding this comment.
It looks like I cannot use keyboard_layout without display_type being set. E.g. if I set keyboard_layout only, it will be ignored and not passed into params under ['host']['compute_attributes']. Is that expected behaviour?
There was a problem hiding this comment.
No, actually only if the display type is VNC there should not be an option to choose the key_board layout according to the web UI. I will fix it.
There was a problem hiding this comment.
After update I see that I can pass display_type or keyboard_layout independently 👍. But there is no mention that a user shouldn't pass keyboard_layout if he/she passes display_type = VNC. Honestly I don't know if it is a problem, my concern is based on your previous comment only.
If we want to address it as well, then I'd expand a bit the help: 'keyboard_layout', _('Possible values: %s. Not usable if display type is VNC.') %... or something similar.
OR
We could add display type check in the code and warn the user if he/she passes display_type = VNC and keyboard_layout = anything.
What do you think? Otherwise the PR is ready to merge.
There was a problem hiding this comment.
I think expanding the help is a good idea.
I also made a mistake with what I wrote. Actually it is possible to add keybaord_layout only when the display_type is VNC. When the display_type is SPICE (the other option) it should not be added.
I will add a short explanation to the help. :)
Thanks!
No description provided.