Nautobot changed vrf_id -> vrfs and tag -> tags in 2.x #68
Conversation
Reviewer's Guide by SourceryThis pull request addresses the divergence in API between NetBox and Nautobot with the release of version 2.x. The changes include detecting the version of the API and updating the filtering logic to use the new field names (vrfs and tags) when the version is 2.x. File-Level Changes
Tips
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @dethmetaljeff - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
octodns_netbox/__init__.py
Outdated
| self._nb_version = "1.x" | ||
| status = self._nb_client.status() | ||
| if "netbox-version" in status: | ||
| self._nb_version = status["netbox-version"] | ||
| elif "nautobot-version" in status: | ||
| self._nb_version = status["nautobot-version"] | ||
| else: | ||
| self.log.warning("Failed to detect NetBox version, assuming 1.x") |
There was a problem hiding this comment.
suggestion (bug_risk): Consider making version detection more robust
The current implementation assumes '1.x' if neither 'netbox-version' nor 'nautobot-version' is found. This could lead to issues if the actual version is different. Consider adding more robust error handling or a fallback mechanism.
| self._nb_version = "1.x" | |
| status = self._nb_client.status() | |
| if "netbox-version" in status: | |
| self._nb_version = status["netbox-version"] | |
| elif "nautobot-version" in status: | |
| self._nb_version = status["nautobot-version"] | |
| else: | |
| self.log.warning("Failed to detect NetBox version, assuming 1.x") | |
| self._nb_version = None | |
| status = self._nb_client.status() | |
| if "netbox-version" in status: | |
| self._nb_version = status["netbox-version"] | |
| elif "nautobot-version" in status: | |
| self._nb_version = status["nautobot-version"] | |
| else: | |
| self.log.error("Failed to detect NetBox version, cannot proceed") | |
| raise ValueError("Unsupported NetBox version") |
octodns_netbox/__init__.py
Outdated
| vrf_filter_field = "vrfs" if self._nb_version.startswith("2.") else "vrf_id" | ||
| tag_filter_field = "tags" if self._nb_version.startswith("2.") else "tag" | ||
| kw = { | ||
| f"{self.field_name}__empty": "false", | ||
| f"{vrf_filter_field}": self.populate_vrf_id, | ||
| f"{tag_filter_field}": self.populate_tags, |
There was a problem hiding this comment.
suggestion (performance): Potential performance impact due to repeated version checks
The repeated use of 'self._nb_version.startswith("2.")' in multiple places could have a performance impact. Consider storing the result of this check in a variable to avoid redundant computations.
| vrf_filter_field = "vrfs" if self._nb_version.startswith("2.") else "vrf_id" | |
| tag_filter_field = "tags" if self._nb_version.startswith("2.") else "tag" | |
| kw = { | |
| f"{self.field_name}__empty": "false", | |
| f"{vrf_filter_field}": self.populate_vrf_id, | |
| f"{tag_filter_field}": self.populate_tags, | |
| is_version_2 = self._nb_version.startswith("2.") | |
| vrf_filter_field = "vrfs" if is_version_2 else "vrf_id" | |
| tag_filter_field = "tags" if is_version_2 else "tag" | |
| kw = { | |
| f"{self.field_name}__empty": "false", | |
| f"{vrf_filter_field}": self.populate_vrf_id, | |
| f"{tag_filter_field}": self.populate_tags, |
octodns_netbox/__init__.py
Outdated
| tag_filter_field = "tags" if self._nb_version.startswith("2.") else "tag" | ||
| kw = { | ||
| f"{self.field_name}__empty": "false", | ||
| f"{vrf_filter_field}": self.populate_vrf_id, | ||
| f"{tag_filter_field}": self.populate_tags, |
There was a problem hiding this comment.
nitpick: Inconsistent naming conventions for filter fields
The naming conventions for 'vrf_filter_field' and 'tag_filter_field' are inconsistent. Consider standardizing the naming to improve code readability and maintainability.
The API between nautobot and netbox slightly diverge with the release of 2.x. This fixes that.
Summary by Sourcery
This pull request updates the code to handle API differences between NetBox and Nautobot versions 1.x and 2.x by adding version detection and adjusting the field names for VRF and tags accordingly.