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

_get_initial_value_order seems to be flawed #209

Open
milan2655 opened this issue Feb 22, 2023 · 11 comments
Open

_get_initial_value_order seems to be flawed #209

milan2655 opened this issue Feb 22, 2023 · 11 comments
Labels
status: accepted This issue has been accepted by the maintainers team for implementation type: bug

Comments

@milan2655
Copy link

milan2655 commented Feb 22, 2023

Environment

  • DiffSync version: 1.7.0
  • Python version 3.10.6

Output of the method get_tree_traversal() doesn't show correct order.

NautobotAdapter
── site
│   ├── device
│   │   └── interface
│   │       └── aip_address
│── aip_address

if the ip_class doesn't start with "a", output ios ok.
 
NautobotAdapter
── site
│   ├── device
│   │   └── interface
│   │       └── ip_address

BR Milan

@itdependsnetworks
Copy link
Contributor

Can you show what your DiffSync class looks like?

@milan2655
Copy link
Author

milan2655 commented Feb 22, 2023

class NautobotAdapter(DiffSync):
    site = Site
    device = Device
    interface = Interface
    aip_address = IpAddress

   top_level = ["site"]

@Kircheneer
Copy link
Collaborator

Can you try to come up with a minimal reproduction of this issue (i.e. does it always occur when a model name start with a?) and post the code in this issue that does that? Then we can troubleshoot it quickly and easily. Thanks!

@Kircheneer Kircheneer added status: action required This issue requires additional information to be actionable type: bug labels Feb 22, 2023
@milan2655
Copy link
Author

test.py.zip

hallo,
test file is attached. changing modelname from _modelname = "aip_address" to _modelname = "ip_address", change the output of the metod get_tree_traversal().
br milan

@Kircheneer Kircheneer added status: accepted This issue has been accepted by the maintainers team for implementation and removed status: action required This issue requires additional information to be actionable labels Feb 23, 2023
@itdependsnetworks
Copy link
Contributor

Your children are mapping to ip_address and not aip_address.

class Interface(DiffSyncModel):
    _children = {
        "ip_address": "ip_addresses",
    }

@milan2655
Copy link
Author

milan2655 commented Feb 23, 2023

i have changed mapping to aip_address:

_children = {
    "aip_address": "ip_addresses",
}

and the output is not changed:

NautobotAdapter
├── site
│   └── device
│       └── interface
│           └── aip_address
└── aip_address

@itdependsnetworks
Copy link
Contributor

Please show the full output, the issues can only be shown then. The prior as an example had clear issues, perhaps there are new ones.

@milan2655
Copy link
Author

how do you mean full output?

@itdependsnetworks itdependsnetworks changed the title get_tree_traversal _get_initial_value_order seems to be flawed Feb 24, 2023
@itdependsnetworks
Copy link
Contributor

I think I see the issue. I have updated the title. _get_initial_value_order seems to not take into consideration children beyond top level, so by the time it get's to children of children there is not a correct expectation of what order it should be processed in

@milan2655
Copy link
Author

did you fix the issue?

@Kircheneer
Copy link
Collaborator

did you fix the issue?

No, the issue is not fixed at this time. However, do feel free to submit a PR with a fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: accepted This issue has been accepted by the maintainers team for implementation type: bug
Projects
None yet
Development

No branches or pull requests

3 participants