Skip to content

Add new example with pynautobot - #53

Merged
dgarros merged 25 commits into
mainfrom
dga-examples-nautobot
Oct 15, 2021
Merged

Add new example with pynautobot#53
dgarros merged 25 commits into
mainfrom
dga-examples-nautobot

Conversation

@dgarros

Copy link
Copy Markdown
Contributor

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

Comment threadexamples/example3/README.md Outdated
Comment threadexamples/example3/README.md Outdated
Comment threadexamples/example3/README.md Outdated
Comment threadexamples/example3/README.md Outdated
Comment threadexamples/example3/local_adapter.py
Comment threadexamples/example3/nautobot_adapter.py Outdated
Comment threadexamples/example3/nautobot_adapter.py Outdated
Comment threadexamples/example3/nautobot_adapter.py Outdated
Comment threadexamples/example3/nautobot_adapter.py Outdated
Comment threadexamples/example3/nautobot_adapter.py Outdated
@dgarros
dgarrosforce-pushed the dga-examples-nautobot branch from 9b4c2f1 to 1e6376cCompareOctober 8, 2021 19:51
@dgarros
dgarros changed the base branch from master to mainOctober 8, 2021 20:04
@dgarros
dgarrosforce-pushed the dga-examples-nautobot branch from 9f56d1f to 5288b5fCompareOctober 11, 2021 13:04

@glennmatthewsglennmatthews left a comment

Copy link
Copy Markdown
Collaborator

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.

Comment threadexamples/example3/diff.py Outdated
Comment threadexamples/example3/nautobot_models.py
Comment threadtasks.py
# 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
Copy Markdown
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
Copy Markdown
ContributorAuthor

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
Copy Markdown
ContributorAuthor

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

Comment threadtasks.py Outdated
# 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
Copy Markdown
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
Copy Markdown
ContributorAuthor

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
Copy Markdown
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?

Comment threadexamples/example3/main.py
dgarrosand 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>
dgarrosand 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 mainOct 15, 2021
Sign up for freeto 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

@dgarros@glennmatthews