netbox-community / pynetbox

Python API client library for Netbox.
Apache License 2.0
581 stars 173 forks source link

Not possible to pickle objects #329

Closed berahtlv closed 3 years ago

berahtlv commented 3 years ago

On pynetbox version higher than 5.0.8 it is not possible to pickle objects. Used for testing purposes in absence of Netbox.

pynetbox-5.3.1/pynetbox-5.1.1:

nbox = pynetbox.api(NBOX_URL, token=NBOX_TOKEN) sites = nbox.dcim.sites.all() f = open("object.pickle", "wb") pickle.dump(sites, f) Traceback (most recent call last): File "", line 1, in TypeError: 'App' object is not callable

pynetbox-5.0.8:

nbox = pynetbox.api(NBOX_URL, token=NBOX_TOKEN) sites = nbox.dcim.sites.all() f = open("object.pickle", "wb") pickle.dump(sites, f) f.close()

markkuleinio commented 3 years ago

This is my attempted fix in PluginsApp in app.py:

    def __getstate__(self):
        return self.__dict__

    def __setstate__(self, d):
        self.__dict__.update(d)

(and it makes pickle.dump() work for me, even though I've never worked with pickle so YMMV)

but pytest does not work due to:

...
tests/integration/conftest.py:6: in <module>
    import yaml
E   ModuleNotFoundError: No module named 'yaml'

@raddessi or @zachmoody, is there something specific we should know nowadays when contributing? I've seen you developing the testing setup lately.

raddessi commented 3 years ago

We're currently testing adding an integration suite to the project but unfortunately I think my first commit was not very stable and broke not long after netbox-docker was updated to 1.0.. I apologize. I think I have those issues sorted in https://github.com/digitalocean/pynetbox/pull/327 but it still needs a look over. As far as I know integration tests aren't needed yet but Zach will have to weigh in on path forward for now. I'm pretty sure this project should still support python2 but I'm not 100% sure to what extent.

raddessi commented 3 years ago

@berahtlv What version of python are you running? You should be able to run the unit tests using python 3 without error, but the integration tests will fail right now I think. Try running python3 -m pytest tests/test* tests/unit/*

berahtlv commented 3 years ago

I am running python 3.6.8 and Netbox v2.9.3. Issue is related to App instance pickling. Yes, tests python3 -m pytest tests/test* tests/unit/* are successful.

markkuleinio commented 3 years ago

@raddessi @zachmoody Note that the master branch is still broken:

(clean-venv)$ pytest
================================ test session starts =================================
platform linux -- Python 3.7.3, pytest-6.2.2, py-1.10.0, pluggy-0.13.1
...
tests/integration/conftest.py:6: in <module>
    import yaml
E   ModuleNotFoundError: No module named 'yaml'

AFAIK yaml is not listed as a requirement anywhere. Should you be developing the integration tests in some other branch, or what do you suggest for other contributors to use as a base in this situation (or did I miss some change in contribution guidelines)?

raddessi commented 3 years ago

Oh my apologies, for some reason I thought yaml was builtin in python 3.x now. We should probably add a requirements-dev.txt with yaml and the rest of the dev requirements listed in there in that case right?

markkuleinio commented 3 years ago

The PR (#331) is currently passing the checks (in GitHub), so let's see if @zachmoody will accept it.

zachmoody commented 3 years ago

Yeah, for sure, just merged it. Been trying to get things together for the 6.0 release. Wanted to put this one in the last 5.x release, but I'd already merged some of the breaking changes before I remembered this one was still outstanding 😬