-
Notifications
You must be signed in to change notification settings - Fork 105
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 omit keyword usage in j2 templates #2438
base: main
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Thanks for the PR! ❤️ |
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/958b59c4aad446db84155323f01d8b40 ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 30m 07s |
925e00b
to
0fe66cc
Compare
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.
Thank you for fixing this one @pablintino
I have just left a query.
iface: {{ omit if ns.interfaces[network.network_name] is not defined else ns.interfaces[network.network_name] }} | ||
{% else %} | ||
iface: {{ omit if ns.interfaces[network.network_name] is not defined else network.network_name }} | ||
iface: {{ network.network_name }} |
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.
ditto
iface: {{ omit if ns.interfaces[network.network_name] is not defined else ns.interfaces[network.network_name] }} | ||
{% else %} | ||
iface: {{ omit if ns.interfaces[network.network_name] is not defined else network.network_name }} | ||
iface: {{ network.network_name }} |
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 seems like a change in behavior, if it's intended we should list it in the commit message. Should ns.interfaces[network.network_name]
be checked before setting iface or was this a typo in the original patch 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.
I ask because on line 73 you check the state of ns.interfaces[network.network_name]
iface: {{ omit if ns.interfaces[network.network_name] is not defined else ns.interfaces[network.network_name] }} | ||
{% else %} | ||
iface: {{ omit if ns.interfaces[network.network_name] is not defined else network.network_name }} | ||
iface: {{ network.network_name }} |
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.
ditto
No description provided.