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

Add new example with pynautobot #53

Merged
merged 25 commits into from
Oct 15, 2021
Merged

Add new example with pynautobot #53

merged 25 commits into from
Oct 15, 2021

Conversation

dgarros
Copy link
Contributor

@dgarros dgarros commented Jun 18, 2021

This PR adds a new example to show how to use DiffSync to interact with a remote system via its API, like Nautobot.

examples/example3/README.md Outdated Show resolved Hide resolved
examples/example3/README.md Outdated Show resolved Hide resolved
examples/example3/README.md Outdated Show resolved Hide resolved
examples/example3/README.md Outdated Show resolved Hide resolved
examples/example3/local_adapter.py Show resolved Hide resolved
examples/example3/nautobot_adapter.py Outdated Show resolved Hide resolved
examples/example3/nautobot_adapter.py Outdated Show resolved Hide resolved
examples/example3/nautobot_adapter.py Outdated Show resolved Hide resolved
examples/example3/nautobot_adapter.py Outdated Show resolved Hide resolved
examples/example3/nautobot_adapter.py Outdated Show resolved Hide resolved
@dgarros dgarros changed the base branch from master to main October 8, 2021 20:04
Copy link
Collaborator

@glennmatthews glennmatthews left a comment

Choose a reason for hiding this comment

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

Thanks for the updates! A few more comments, plus I saw that you'd marked quite a few of my previous suggestions as resolved without any corresponding code changes, so I've unresolved the ones that appear still relevant.

examples/example3/diff.py Outdated Show resolved Hide resolved
examples/example3/nautobot_models.py Show resolved Hide resolved
@@ -197,7 +197,7 @@ def mypy(context, name=NAME, image_ver=IMAGE_VER, local=INVOKE_LOCAL):
"""
# pty is set to true to properly run the docker commands due to the invocation process of docker
# https://docs.pyinvoke.org/en/latest/api/runners.html - Search for pty for more information
exec_cmd = 'find . -name "*.py" | xargs mypy --show-error-codes'
exec_cmd = 'find . -name "*.py" -not -path "*/examples/*" -not -path "*/docs/*" | xargs mypy --show-error-codes'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not run mypy against the example code? Seems like it'd be good practice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Similar to pylint, mypy was complaining that we have Duplicate module between examples.
looks like it's a known limitation python/mypy#4008

LOCAL - Running command find . -name "*.py" | xargs mypy --show-error-codes
examples/example1/models.py: error: Duplicate module named 'models' (also at './examples/example3/models.py')  [misc]
Found 1 error in 1 file (checked 29 source files)
Error: Process completed with exit code 123.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was able to reactivate pylint but based on our discussion i would prefer to keep mypy disabled for now to avoid renaming the files

tasks.py Outdated
@@ -213,7 +213,7 @@ def pylint(context, name=NAME, image_ver=IMAGE_VER, local=INVOKE_LOCAL):
"""
# pty is set to true to properly run the docker commands due to the invocation process of docker
# https://docs.pyinvoke.org/en/latest/api/runners.html - Search for pty for more information
exec_cmd = 'find . -name "*.py" | xargs pylint'
exec_cmd = 'find . -name "*.py" -not -path "*/examples/*" -not -path "*/docs/*" | xargs pylint'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not run pylint against the example code? Seems like it'd be good practice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is the list of errors reported by pylint when it's enabled.
It feels like pylint is expecting all python code to be part of the same package and it is not happy because we have some redundant code and library across some examples.

DOCKER - Running command: find . -name "*.py" | xargs pylint container: diffsync-py3.7:1.3.0
************* Module diff
examples/example3/diff.py:11:24: W0612: Unused variable 'child' (unused-variable)
************* Module main
examples/example3/main.py:9:0: C0411: third party import "from local_adapter import LocalAdapter" should be placed before "from diffsync.enum import DiffSyncFlags" (wrong-import-order)
examples/example3/main.py:10:0: C0411: third party import "from nautobot_adapter import NautobotAdapter" should be placed before "from diffsync.enum import DiffSyncFlags" (wrong-import-order)
examples/example3/main.py:11:0: C0411: third party import "from diff import AlphabeticalOrderDiff" should be placed before "from diffsync.enum import DiffSyncFlags" (wrong-import-order)
************* Module local_adapter
examples/example3/local_adapter.py:4:0: E0401: Unable to import 'slugify' (import-error)
examples/example3/local_adapter.py:28:4: W0221: Parameters differ from overridden 'load' method (arguments-differ)
************* Module nautobot_models
examples/example3/nautobot_models.py:2:0: E0401: Unable to import 'pynautobot' (import-error)
examples/example3/nautobot_models.py:86:33: E1101: Instance of 'NautobotCountry' has no 'slug' member (no-member)
examples/example3/nautobot_models.py:5:0: C0411: third party import "from models import Region, Country" should be placed before "from diffsync import DiffSync" (wrong-import-order)
************* Module nautobot_adapter
examples/example3/nautobot_adapter.py:3:0: E0401: Unable to import 'pynautobot' (import-error)
examples/example3/nautobot_adapter.py:79:4: W0222: Signature differs from overridden 'sync_from' method (signature-differs)
examples/example3/nautobot_adapter.py:47:8: W0201: Attribute 'nautobot' defined outside __init__ (attribute-defined-outside-init)
************* Module backend_b
examples/example1/backend_b.py:20:0: E0611: No name 'Site' in module 'models' (no-name-in-module)
examples/example1/backend_b.py:20:0: E0611: No name 'Device' in module 'models' (no-name-in-module)
examples/example1/backend_b.py:20:0: E0611: No name 'Interface' in module 'models' (no-name-in-module)
************* Module backend_c
examples/example1/backend_c.py:20:0: E0611: No name 'Site' in module 'models' (no-name-in-module)
examples/example1/backend_c.py:20:0: E0611: No name 'Device' in module 'models' (no-name-in-module)
examples/example1/backend_c.py:20:0: E0611: No name 'Interface' in module 'models' (no-name-in-module)
************* Module backend_a
examples/example1/backend_a.py:20:0: E0611: No name 'Site' in module 'models' (no-name-in-module)
examples/example1/backend_a.py:20:0: E0611: No name 'Device' in module 'models' (no-name-in-module)
examples/example1/backend_a.py:20:0: E0611: No name 'Interface' in module 'models' (no-name-in-module)

https://github.com/networktocode/diffsync/runs/3861062794

Copy link
Collaborator

Choose a reason for hiding this comment

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

Interesting. Does renaming the duplicated models.py files to example1_models.py and example3_models.py respectively help to fix this?

examples/example3/main.py Show resolved Hide resolved
dgarros and others added 13 commits October 12, 2021 10:19
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
dgarros and others added 7 commits October 12, 2021 16:16
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
@dgarros dgarros merged commit 04890dc into main Oct 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants