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

fix: ipam interface should check on virtualmachine nil state #34

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

PeterO2
Copy link

@PeterO2 PeterO2 commented Feb 13, 2020

Class IpAddress has a function called 'interface' returning a Virtualization or DCIM interface based on the existance of the 'virtual_machine' property. But in case of a DCIM interface this property exists as well, only set to nil. So this nil check has to be added to return the right interface.

@PeterO2 PeterO2 force-pushed the fix_wrong_ipaddress_interface branch from af7e7fa to 77ab597 Compare February 16, 2020 22:32
@thde thde force-pushed the fix_wrong_ipaddress_interface branch from bd395b5 to b0b8a9c Compare October 28, 2024 11:42
@thde thde changed the title Fix: ipam interface should check on virtualmachine nil state fix: ipam interface should check on virtualmachine nil state Oct 28, 2024
thde
thde previously approved these changes Oct 28, 2024
@@ -34,7 +34,7 @@ def interface

return nil unless interface_data

if interface_data.key?('virtual_machine')
if interface_data.key? ('virtual_machine') && !interface_data['virtual_machine'].nil?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Be careful of the space between key? and ('virtual_machine')

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the implementation to a, in my opinion, simpler one.

@thde thde force-pushed the fix_wrong_ipaddress_interface branch from b0b8a9c to 46692ca Compare October 29, 2024 09:27
@thde thde merged commit bf4a644 into ninech:master Oct 29, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants