Skip to content

Add location_parent_parent_name etc support#502

Open
cdtomkins wants to merge 18 commits intonautobot:developfrom
cdtomkins:ctomkins_location_grandparent_support
Open

Add location_parent_parent_name etc support#502
cdtomkins wants to merge 18 commits intonautobot:developfrom
cdtomkins:ctomkins_location_grandparent_support

Conversation

@cdtomkins
Copy link
Copy Markdown
Contributor

@cdtomkins cdtomkins commented Jan 28, 2026

Closes #399

What's Changed

Replaces the previous single-parent lookup logic with a dynamic ancestry traversal that supports arbitrary depth in the location hierarchy.

Before

  • Only location_name plus an optional location_parent_name were valid
  • This could lead to scenarios where uniqueness could not be guaranteed and devices could not be onboarded via CSV.

After

  • The new _get_location_by_dynamic_ancestry() method discovers all location_parent* columns in the CSV row, orders them by depth, and walks the hierarchy from root to immediate parent before resolving the target Location.
  • This makes the import logic resilient to deeply nested structures (e.g. Region → Campus → Building → Floor → Room).
  • Even repeated names in the path (e.g. a building and campus with the same name) works fine.
  • Also improves error handling/logging.

To Do

  • Explanation of Change(s)
  • Added change log fragment(s)
  • Unit, Integration Tests

Comment thread nautobot_device_onboarding/jobs.py
Comment thread nautobot_device_onboarding/jobs.py Outdated
Copy link
Copy Markdown

@DistantVoyager DistantVoyager left a comment

Choose a reason for hiding this comment

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

@cdtomkins take a look at my suggestions. I have not tested, but please do and see if that (or something similar) will handle n number of location_parents from a CSV

@cdtomkins cdtomkins changed the title Add location_parent_parent_name support Add location_parent_parent_name etc support Feb 20, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Feb 20, 2026

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  nautobot_device_onboarding
  jobs.py 453, 468-470
Project Total  

This report was generated by python-coverage-comment-action

@cdtomkins cdtomkins self-assigned this Feb 20, 2026
@cdtomkins
Copy link
Copy Markdown
Contributor Author

@Dav-C @scetron Last week we discussed two possibilities for this one:

  1. add unit tests and push forward with this as-is, then raise a separate, new issue to standardise the column names to dunder notation
  2. standardise the column names to dunder notation as part of this PR

Personally I think (1) is the right path and align it with next major version. Thoughts appreciated. If you agree (1) I'll get the UTs finished up.

@PiotrOrman
Copy link
Copy Markdown

Hello,
When it will be public available ? Looks like it is going to cover Bug #399 raised by me. Do you need any help with testing ?

@cdtomkins
Copy link
Copy Markdown
Contributor Author

Hello, When it will be public available ? Looks like it is going to cover Bug #399 raised by me. Do you need any help with testing ?

Yes I agree it should close #399. This patch is working fine, I just need to write some tests and then I should be able to merge. I hope to do so this week.

If you have a development environment, feel free to test there in the meantime.

@cdtomkins cdtomkins marked this pull request as ready for review March 18, 2026 09:46
@cdtomkins
Copy link
Copy Markdown
Contributor Author

@Dav-C made a good recommendation to make this more performant by avoiding repeat lookups where a row's location ancestry has already been checked by adding a cache. Will action this before merge.

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.

Multiple locations with the same name (Vlan creation)

3 participants