Skip to content

Upgrading to Pydantic v2 and DiffSync v2 - #327

Open
theandrelima wants to merge 11 commits into
networktocode:developfrom
theandrelima:theandrelima_moving_pydantic2x
Open

Upgrading to Pydantic v2 and DiffSync v2#327
theandrelima wants to merge 11 commits into
networktocode:developfrom
theandrelima:theandrelima_moving_pydantic2x

Conversation

@theandrelima

@theandrelima theandrelima commented Jul 4, 2024

Copy link
Copy Markdown

Overview

network-importer's dependency on pydantic = "^1.6" limits users who need to use other packages dependent on Pydantic 2.x in the same Python environment.

The original motivation behind this PR was to move network-importer's dependency to Pydantic v2.x. However, network-importer depends on DiffSync, which in turn also depends on Pydantic.

For that, moving to DiffSync v2 is also required for compatibility with Pydantic v2.

Changes Made

  • Upgraded to Pydantic v2.
  • Refactored settings classes to use pydantic-settings.
  • Applied overall code fixes for Pydantic v2 compatibility.
  • Upgraded to DiffSync v2 and made necessary code adjustments for breaking changes.
  • Proposed project version bump to 4.0.0 (subject to NTC's discretion) due to breaking changes.

Testing

  • All tests passed using the invoke pytest command. (see Additional Notes below)
  • Manual testing in CLI mode against a dev instance of our Nautobot environment confirmed the functionality both in check and apply modes, and with/without --update-configs and --limit flags.

Additional Notes

invoke pytest only worked after I added the following line to the end of the Dockerfile: RUN pip install .

I'm not sure why, but lines 14 and 15 of the current Dockerfile were not taking the effect of making network_importer itself available in the python environment of the resulting built image.

I decided to not tackle this at all within this PR because:
1 - It might be some weirdness with my own sandbox environment.
2 - Even if it is not, I believe it is out-of-scope.

@msheiny

msheiny commented Dec 24, 2024

Copy link
Copy Markdown

Looks like this is failing because of the removed docstrings in BatfishSettings and NetworkSettings classes:

./network_importer/config.py:41 in public class `BatfishSettings`:
        D101: Missing docstring in public class
./network_importer/config.py:51 in public class `NetworkSettings`:
        D101: Missing docstring in public class
@jvanderaa

Copy link
Copy Markdown
Contributor

I'm onboard with getting this merged in and let's then take a pass at getting the rest updated.

For that update (not this one), @theandrelima would we be good to update Python minimums in your environment? Only because Pandas and Numpy are aggressively moving forward.

@theandrelima

theandrelima commented Jan 1, 2025

Copy link
Copy Markdown
Author

I'm onboard with getting this merged in and let's then take a pass at getting the rest updated.

For that update (not this one), @theandrelima would we be good to update Python minimums in your environment? Only because Pandas and Numpy are aggressively moving forward.

I was off for a couple of weeks. Back now and happy to see that this might get merged soon, hopefully?

All right! tWill do.
But just to confirm, that is to happen in a separate PR, right?

@theandrelima

theandrelima commented May 19, 2025

Copy link
Copy Markdown
Author

It seems the idea is to deprecate network-importer as a standalone package in favour of the new onboarding plugin, which will include network importing functionality too.

Whereas that is nice, I'm a big proponent of an independent NI package that can be integrated in whichever way best suits each use case. For example, I currently would love to have NI as a service with a backend REST API, dedicated workers, and running independently of Netbox/Nautobot app, while updating it.

I'll keep my own version of this package, possibly with a different name too. Hope that's not a problem...

cheers...
🍻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants