Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CI & linters. #75

Merged
merged 7 commits into from
Jul 1, 2022
Merged

Fix CI & linters. #75

merged 7 commits into from
Jul 1, 2022

Conversation

pszulczewski
Copy link
Contributor

Bumped up python version to 3.7, refreshed poetry.lock + few linting updates.

Unittests are failing during setUpClass with the following exception.

  File "/usr/local/lib/python3.7/site-packages/diffsync/store/__init__.py", line 219, in _get_uid
    f"Invalid args: ({model}, {identifier}): "
ValueError: Invalid args: (customfieldchoice, {'field': {'name': 'Custom Choices Field'}, 'value': 'alpha'}): either customfieldchoice should be a class/instance or {'field': {'name': 
'Custom Choices Field'}, 'value': 'alpha'} should be a str

This is beyond my diffsync knowledge.

@pszulczewski
Copy link
Contributor Author

Pinning diffsync back to 1.3.0 resolve methods signature incompatibility in 1.5.0, linting is now fine, however unittests are failing due to maximum recursion depth exceeded in deepcopy.

Running unit tests...
<...>
.....EE...
======================================================================
ERROR: test_map_object_type (nautobot_netbox_importer.tests.test_import_objectchange.TestImportObjectChangeMethods)
Validate map_object_type method.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/source/nautobot_netbox_importer/tests/test_import_objectchange.py", line 92, in test_map_object_type
    assert self.cmd.map_object_type(key, entry, set()) is True
  File "/usr/local/lib/python3.7/site-packages/django/test/testcases.py", line 1124, in __get__
    data = deepcopy(self.data, memo)
  File "/usr/local/lib/python3.7/copy.py", line 180, in deepcopy
    y = _reconstruct(x, memo, *rv)
  File "/usr/local/lib/python3.7/copy.py", line 281, in _reconstruct
    state = deepcopy(state, memo)
  File "/usr/local/lib/python3.7/copy.py", line 150, in deepcopy
    y = copier(x, memo)
  File "/usr/local/lib/python3.7/copy.py", line 241, in _deepcopy_dict
    y[deepcopy(key, memo)] = deepcopy(value, memo)
  File "/usr/local/lib/python3.7/copy.py", line 180, in deepcopy
    y = _reconstruct(x, memo, *rv)
  File "/usr/local/lib/python3.7/copy.py", line 282, in _reconstruct
    if hasattr(y, '__setstate__'):
  File "/usr/local/lib/python3.7/site-packages/django/core/management/base.py", line 141, in __getattr__
    return getattr(self._out, name)
  File "/usr/local/lib/python3.7/site-packages/django/core/management/base.py", line 141, in __getattr__
    return getattr(self._out, name)
  File "/usr/local/lib/python3.7/site-packages/django/core/management/base.py", line 141, in __getattr__
    return getattr(self._out, name)
  [Previous line repeated 4977 more times]
RecursionError: maximum recursion depth exceeded

======================================================================
ERROR: test_process_objectchange (nautobot_netbox_importer.tests.test_import_objectchange.TestImportObjectChangeMethods)
Validate process_objectchange method.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/source/nautobot_netbox_importer/tests/test_import_objectchange.py", line 109, in test_process_objectchange
    self.cmd.process_objectchange(entry, set())
  File "/usr/local/lib/python3.7/site-packages/django/test/testcases.py", line 1124, in __get__
    data = deepcopy(self.data, memo)
  File "/usr/local/lib/python3.7/copy.py", line 180, in deepcopy
    y = _reconstruct(x, memo, *rv)
  File "/usr/local/lib/python3.7/copy.py", line 281, in _reconstruct
    state = deepcopy(state, memo)
  File "/usr/local/lib/python3.7/copy.py", line 150, in deepcopy
    y = copier(x, memo)
  File "/usr/local/lib/python3.7/copy.py", line 241, in _deepcopy_dict
    y[deepcopy(key, memo)] = deepcopy(value, memo)
  File "/usr/local/lib/python3.7/copy.py", line 180, in deepcopy
    y = _reconstruct(x, memo, *rv)
  File "/usr/local/lib/python3.7/copy.py", line 282, in _reconstruct
    if hasattr(y, '__setstate__'):
  File "/usr/local/lib/python3.7/site-packages/django/core/management/base.py", line 141, in __getattr__
    return getattr(self._out, name)
  File "/usr/local/lib/python3.7/site-packages/django/core/management/base.py", line 141, in __getattr__
    return getattr(self._out, name)
  File "/usr/local/lib/python3.7/site-packages/django/core/management/base.py", line 141, in __getattr__
    return getattr(self._out, name)
  [Previous line repeated 4977 more times]
RecursionError: maximum recursion depth exceeded

----------------------------------------------------------------------
Ran 10 tests in 101.878s

FAILED (errors=2)

@chadell
Copy link
Contributor

chadell commented Jun 30, 2022

@pszulczewski I'm getting a similar issue here: https://github.com/networktocode/circuit-maintenance-parser/runs/7113884836?check_suite_focus=true
It works well in my local dev environment, but it fails in the CI
maybe it's something new in copy.deepcopy?

@pszulczewski
Copy link
Contributor Author

Nope, it fails on my PC(AMD 4750U) WIN10 + WSL2 Ubuntu. Max recursion by default is set to 1000, setting it higher to 10.000 doesn't solve it.
I am looking if there was any change from py3.6 going up to 3.7 but haven't found anything meaningful yet.

@glennmatthews
Copy link
Contributor

Initial investigation is suggesting that it may be an issue introduced in recent versions of Pydantic (such as pydantic/pydantic#4114 perhaps?) but I'll continue to look into it.

@glennmatthews
Copy link
Contributor

The original reported ValueError should be fixed by networktocode/diffsync#118; still investigating the RecursionError.

Comment on lines -53 to +58
def setUpTestData(cls) -> None:
"""One-time setup function called before running the test functions in this class."""
cls.cmd = CommandObjChange()
cls.cmd.logger = Mock()
cls.cmd.logger.warning = Mock()
def setUp(self) -> None:
"""Pre-test setup function.

Note that this was originally a setUpTestData() classmethod, but when moving to a newer version of Django,
we found that then accessing `self.cmd` would result in Django throwing an infinite RecursionError.
Probably a Django bug, but changing to setUp() appears to avoid the issue at the cost of slightly slower tests.
"""
Copy link
Contributor Author

@pszulczewski pszulczewski Jul 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was really tricky!!!
Thanks Glenn for moving this forward!

Copy link
Contributor

@chadell chadell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants