-
Notifications
You must be signed in to change notification settings - Fork 148
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
Fix reading resource params with nested spec #455
base: master
Are you sure you want to change the base?
Conversation
@ggverma, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding
|
if params: | ||
filtered_params[key] = params | ||
elif current_spec_type == 'list' and v: | ||
if isinstance(v[0], dict): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might deserve a comment for improving readability.
for instance - assuming that if the first element of the list is a dict, then all the other element will be dicts as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, they will be dict though because spec looks like this in such case:
next_hops=dict(
required=True,
type='list', <--------- current_spec_type
elements='dict', <--- elements can be all str, dict, or list. This defaults to str
options=dict(
admin_distance=dict(
type='int',
default=1
),
ip_address=dict(
type='str'
),
scope=dict(
type='list',
elements='str'
)
)
),
I have updated the condition so that the comment is not needed.
@ggverma, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding
|
@ggverma, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding
|
In Policy modules, the resources are nested. Because of this, we read the resource params from the supplied Ansible Playbook and manually validate it with the resource spec. However, we were not reading the resource params correctly if they were nested. This patch fixes that issue. In addition, we convert the new resource params and existing resource params into Ordered Data so that we can compare them and infer if there are any updates with better accuracy Issue: vmware#417 Signed-off-by: Gautam Verma <vermag@vmware.com> Signed-off-by: Gautam Verma <gautam94verma@gmail.com>
Signed-off-by: Ben Rosenqvist <nebb@protonmail.com>
this fixes a problem I had with bridge_profiles in segments not being idempotent, but creates issues with tier0, in which is throws |
In Policy modules, the resources are nested. Because of this, we read the resource params from the supplied Ansible Playbook and manually validate it with the resource spec. However, we were not reading the resource params correctly if they were nested. This patch fixes that issue. In addition, we convert the new resource params and existing resource params into Ordered Data so that we can compare them and infer if there are any updates with better accuracy
Issue: #417