search

vapor-ware / netbox-virtual-circuit-plugin

A plugin for NetBox that supports Virtual Circuit management
GNU General Public License v3.0
57 stars 19 forks source link

Fix NetBox 2.9.3 compatibility issues #38

Closed hoanhan101 closed 4 years ago

hoanhan101 commented 4 years ago

This PR:

edaniszewski commented 4 years ago

The changes themselves look okay here, but just a few questions:

  1. Are all the changes here related to the 2.9.3 compatibility fix? If not, does it make sense to split the different changes into their own PRs for sake of isolation?
  2. If this makes the plugin compatible with 2.9.3, does that mean that it breaks backwards compatibility with any previous versions where the ChangeLoggedModel hadn't moved? If so, we'll need to figure out what the best practices are for pinning to versions of netbox. Generally for python, its setting the version in setup.py's install_requires field, but given that can be loaded more dynamically into netbox, I'm not sure if that necessarily works, so its something to read up on.
hoanhan101 commented 4 years ago

@edaniszewski

Are all the changes here related to the 2.9.3 compatibility fix? If not, does it make sense to split the different changes into their own PRs for sake of isolation?

Yes, I believe all the changes here are related to the 2.9.3 compatibility fix. The NetBox configuration and required login models needs to be happened along with the updated ChangeLoggedModel path.

If this makes the plugin compatible with 2.9.3, does that mean that it breaks backwards compatibility with any previous versions where the ChangeLoggedModel hadn't moved?

That's a good call. The plugin will probably break our fork which is currently at v2.8.5 if one bumps it to 1.1.0. Let's see if I can find what the best practices are for pinning NetBox version. As for now, do you think having a compatibility matrix is good enough as a workaround?

edaniszewski commented 4 years ago

I think it probably is, but maybe just take a quick look around to see if you can quickly find anything on version pinning practices for netbox. If nothing obvious comes up, or the process to do it is more intensive than just some configuration, then yeah, a compatibility matrix is good and we can open an issue to tackle the version pinning separately.

hoanhan101 commented 4 years ago

I'm opening #39 to tackle the version pinning separately as nothing good came up in my finding.