Support using long interface names with IPFabric #309
Conversation
| @@ -83,3 +83,28 @@ def test_data_loading(self): | |||
| self.assertTrue(hasattr(interface, "ip_address")) | |||
| self.assertTrue(hasattr(interface, "subnet_mask")) | |||
| self.assertTrue(hasattr(interface, "type")) | |||
|
|
|||
| @patch("nautobot_ssot.integrations.ipfabric.diffsync.adapter_ipfabric.IP_FABRIC_ELONGATE_INTERFACE_NAME", True) | |||
There was a problem hiding this comment.
Not requiring to swap out but would be good to patch the setting instead of the constant. Should be the same net effect. TBH how we do constants for plugin config kinda negates the lazy loading part of Django settings BUT that pattern is everywhere and we don't document or expect users to relying on lazy loading
There was a problem hiding this comment.
ya, I'm not a fan either, but following convention. As for patching the setting, as far as I know, I would have to patch all of nautobot_ssot settings, which seems a bit excessive; is there another solution?
There was a problem hiding this comment.
Yes it's a patch of all plugin config but all tests should run against the default config so if you pass in just the one toggle that's all that changed.
There was a problem hiding this comment.
Currently tests are being impacted by the settings in PLUGINS_CONFIG so it might be necessary to patch the specific setting. We are planning on addressing that issue in the near future.
There was a problem hiding this comment.
We should fix the tests to be in a state that do not have to have the settings changed.
There was a problem hiding this comment.
I agree and it's on the todo list.
There was a problem hiding this comment.
I disagree on this one, this is testing a setting change so it should be tested from the PLUGIN_CONFIG side. Intercepting the constant definition could mask an improperly defined plugin config in the NautobotAppConfig.
I am okay resolving this item, whatever we do in the future should test making the setting change in nautobot config and not patching elsewhere. Django lazy loading + default values for plugin config + patching the constant that pulls out a value from plugin config could be a false sense of coverage. I would vote this is a feature request into the nautobot testing framework.
There was a problem hiding this comment.
actually, this is testing whatever the value of the constant is since that is what is imported/used. There should be a separate set of tests for constants.py that tests the constants are set correctly
whitej6
force-pushed
the
jacobm/ipfabric/elongate-interface-names
branch
from
1281bf9
to
2ecb5ca
Compare
jmcgill298
force-pushed
the
jacobm/ipfabric/elongate-interface-names
branch
2 times, most recently
from
eaadc6d
to
4b0d3ac
Compare
jmcgill298
force-pushed
the
jacobm/ipfabric/elongate-interface-names
branch
from
4b0d3ac
to
257210b
Compare
jmcgill298
force-pushed
the
jacobm/ipfabric/elongate-interface-names
branch
from
c749641
to
9da16ff
Compare
No description provided.