Skip to content

Conversation

sdelliot
Copy link
Collaborator

Improve VM property checking for falsy values rather than just missing keys in the Vertex vm dictionary. This is the "proper" way to check a dictionary and will also provide more robust checks.

@mitchnegus
Copy link
Member

mitchnegus commented Sep 9, 2025

Food for thought: we could use the setdefault method for most/all of these. It sets the value on a dictionary (and returns the result) if it does not exist in the dictionary already. It is the built-in one-liner for the if/then clauses that we do here, so it seems like it might be the even more "proper" way to do this.

self.vm.setdefault("architecture", "x86_64"):

But also, we started doing this style on the VyOS MC and I didn't think to suggest it then, so also cool with proceeding here.

@mitchnegus
Copy link
Member

Food for thought: we could use the setdefault method for most/all of these. It sets the value on a dictionary (and returns the result) if it does not exist in the dictionary already. It is the built-in one-liner for the if/then clauses that we do here, so it seems like it might be the even more "proper" way to do this.

self.vm.setdefault("architecture", "x86_64"):

But also, we started doing this style on the VyOS MC and I didn't think to suggest it then, so also cool with proceeding here.

Actually, nevermind. This would probably be preferable if we were only checking for key existence, like in the original, but it will not catch falsy values. This looks good to me.

@mitchnegus mitchnegus merged commit de18599 into sandialabs:main Sep 9, 2025
4 checks passed
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

Successfully merging this pull request may close these issues.

2 participants