mirror of
https://github.com/netbox-community/netbox.git
synced 2026-01-11 21:10:29 +01:00
v3.4-beta1: YAML import raises AttributeError exception #7269
Closed
opened 2025-12-29 20:21:02 +01:00 by adam
·
10 comments
No Branch/Tag Specified
main
update-changelog-comments-docs
feature-removal-issue-type
20911-dropdown
20239-plugin-menu-classes-mutable-state
21097-graphql-id-lookups
feature
fix_module_substitution
20923-dcim-templates
20044-elevation-stuck-lightmode
feature-ip-prefix-link
v4.5-beta1-release
20068-import-moduletype-attrs
20766-fix-german-translation-code-literals
20378-del-script
7604-filter-modifiers-v3
circuit-swap
12318-case-insensitive-uniqueness
20637-improve-device-q-filter
20660-script-load
19724-graphql
20614-update-ruff
14884-script
02496-max-page
19720-macaddress-interface-generic-relation
19408-circuit-terminations-export-templates
20203-openapi-check
fix-19669-api-image-download
7604-filter-modifiers
19275-fixes-interface-bulk-edit
fix-17794-get_field_value_return_list
11507-show-aggregate-and-rir-on-api
9583-add_column_specific_search_field_to_tables
v4.5.0
v4.4.10
v4.4.9
v4.5.0-beta1
v4.4.8
v4.4.7
v4.4.6
v4.4.5
v4.4.4
v4.4.3
v4.4.2
v4.4.1
v4.4.0
v4.3.7
v4.4.0-beta1
v4.3.6
v4.3.5
v4.3.4
v4.3.3
v4.3.2
v4.3.1
v4.3.0
v4.2.9
v4.3.0-beta2
v4.2.8
v4.3.0-beta1
v4.2.7
v4.2.6
v4.2.5
v4.2.4
v4.2.3
v4.2.2
v4.2.1
v4.2.0
v4.1.11
v4.1.10
v4.1.9
v4.1.8
v4.2-beta1
v4.1.7
v4.1.6
v4.1.5
v4.1.4
v4.1.3
v4.1.2
v4.1.1
v4.1.0
v4.0.11
v4.0.10
v4.0.9
v4.1-beta1
v4.0.8
v4.0.7
v4.0.6
v4.0.5
v4.0.3
v4.0.2
v4.0.1
v4.0.0
v3.7.8
v3.7.7
v4.0-beta2
v3.7.6
v3.7.5
v4.0-beta1
v3.7.4
v3.7.3
v3.7.2
v3.7.1
v3.7.0
v3.6.9
v3.6.8
v3.6.7
v3.7-beta1
v3.6.6
v3.6.5
v3.6.4
v3.6.3
v3.6.2
v3.6.1
v3.6.0
v3.5.9
v3.6-beta2
v3.5.8
v3.6-beta1
v3.5.7
v3.5.6
v3.5.5
v3.5.4
v3.5.3
v3.5.2
v3.5.1
v3.5.0
v3.4.10
v3.4.9
v3.5-beta2
v3.4.8
v3.5-beta1
v3.4.7
v3.4.6
v3.4.5
v3.4.4
v3.4.3
v3.4.2
v3.4.1
v3.4.0
v3.3.10
v3.3.9
v3.4-beta1
v3.3.8
v3.3.7
v3.3.6
v3.3.5
v3.3.4
v3.3.3
v3.3.2
v3.3.1
v3.3.0
v3.2.9
v3.2.8
v3.3-beta2
v3.2.7
v3.3-beta1
v3.2.6
v3.2.5
v3.2.4
v3.2.3
v3.2.2
v3.2.1
v3.2.0
v3.1.11
v3.1.10
v3.2-beta2
v3.1.9
v3.2-beta1
v3.1.8
v3.1.7
v3.1.6
v3.1.5
v3.1.4
v3.1.3
v3.1.2
v3.1.1
v3.1.0
v3.0.12
v3.0.11
v3.0.10
v3.1-beta1
v3.0.9
v3.0.8
v3.0.7
v3.0.6
v3.0.5
v3.0.4
v3.0.3
v3.0.2
v3.0.1
v3.0.0
v2.11.12
v3.0-beta2
v2.11.11
v2.11.10
v3.0-beta1
v2.11.9
v2.11.8
v2.11.7
v2.11.6
v2.11.5
v2.11.4
v2.11.3
v2.11.2
v2.11.1
v2.11.0
v2.10.10
v2.10.9
v2.11-beta1
v2.10.8
v2.10.7
v2.10.6
v2.10.5
v2.10.4
v2.10.3
v2.10.2
v2.10.1
v2.10.0
v2.9.11
v2.10-beta2
v2.9.10
v2.10-beta1
v2.9.9
v2.9.8
v2.9.7
v2.9.6
v2.9.5
v2.9.4
v2.9.3
v2.9.2
v2.9.1
v2.9.0
v2.9-beta2
v2.8.9
v2.9-beta1
v2.8.8
v2.8.7
v2.8.6
v2.8.5
v2.8.4
v2.8.3
v2.8.2
v2.8.1
v2.8.0
v2.7.12
v2.7.11
v2.7.10
v2.7.9
v2.7.8
v2.7.7
v2.7.6
v2.7.5
v2.7.4
v2.7.3
v2.7.2
v2.7.1
v2.7.0
v2.6.12
v2.6.11
v2.6.10
v2.6.9
v2.7-beta1
Solcon-2020-01-06
v2.6.8
v2.6.7
v2.6.6
v2.6.5
v2.6.4
v2.6.3
v2.6.2
v2.6.1
v2.6.0
v2.5.13
v2.5.12
v2.6-beta1
v2.5.11
v2.5.10
v2.5.9
v2.5.8
v2.5.7
v2.5.6
v2.5.5
v2.5.4
v2.5.3
v2.5.2
v2.5.1
v2.5.0
v2.4.9
v2.5-beta2
v2.4.8
v2.5-beta1
v2.4.7
v2.4.6
v2.4.5
v2.4.4
v2.4.3
v2.4.2
v2.4.1
v2.4.0
v2.3.7
v2.4-beta1
v2.3.6
v2.3.5
v2.3.4
v2.3.3
v2.3.2
v2.3.1
v2.3.0
v2.2.10
v2.3-beta2
v2.2.9
v2.3-beta1
v2.2.8
v2.2.7
v2.2.6
v2.2.5
v2.2.4
v2.2.3
v2.2.2
v2.2.1
v2.2.0
v2.1.6
v2.2-beta2
v2.1.5
v2.2-beta1
v2.1.4
v2.1.3
v2.1.2
v2.1.1
v2.1.0
v2.0.10
v2.1-beta1
v2.0.9
v2.0.8
v2.0.7
v2.0.6
v2.0.5
v2.0.4
v2.0.3
v2.0.2
v2.0.1
v2.0.0
v2.0-beta3
v1.9.6
v1.9.5
v2.0-beta2
v1.9.4-r1
v1.9.3
v2.0-beta1
v1.9.2
v1.9.1
v1.9.0-r1
v1.8.4
v1.8.3
v1.8.2
v1.8.1
v1.8.0
v1.7.3
v1.7.2-r1
v1.7.1
v1.7.0
v1.6.3
v1.6.2-r1
v1.6.1-r1
1.6.1
v1.6.0
v1.5.2
v1.5.1
v1.5.0
v1.4.2
v1.4.1
v1.4.0
v1.3.2
v1.3.1
v1.3.0
v1.2.2
v1.2.1
v1.2.0
v1.1.0
v1.0.7-r1
v1.0.7
v1.0.6
v1.0.5
v1.0.4
v1.0.3-r1
v1.0.3
1.0.0
Labels
Clear labels
beta
breaking change
complexity: high
complexity: low
complexity: medium
needs milestone
netbox
pending closure
plugin candidate
pull-request
severity: high
severity: low
severity: medium
status: accepted
status: backlog
status: blocked
status: duplicate
status: needs owner
status: needs triage
status: revisions needed
status: under review
topic: GraphQL
topic: Internationalization
topic: OpenAPI
topic: UI/UX
topic: cabling
topic: event rules
topic: htmx navigation
topic: industrialization
topic: migrations
topic: plugins
topic: scripts
topic: templating
topic: testing
type: bug
type: deprecation
type: documentation
type: feature
type: housekeeping
type: translation
Mirrored from GitHub Pull Request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: starred/netbox#7269
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @peteeckel on GitHub (Nov 22, 2022).
Originally assigned to: @arthanson on GitHub.
NetBox version
v3.4-beta1
Python version
3.8
Steps to Reproduce
Expected Behavior
The data is imported and a new IP address appears in the database
Observed Behavior
An
AttributeErrorexception is raised:@jeremystretch commented on GitHub (Nov 22, 2022):
I'm curious whether the expectation from the user perspective is to provide a list of YAML dictionaries, or separate YAML documents representing each object. The YAML loader expects a list of individual documents (separated with
---).@peteeckel commented on GitHub (Nov 22, 2022):
Obviously my expectation was to be able to provide the data in a single YAML array :-)
Anyway, after looking at the code I also tried
(with various different locations of the separators). Always with the same result.
The problem seems to originate in the fact that whether you provide a single YAML document or a list of multiple documents (of which a list containing one document as I did in my minimalised exaxmple),
yaml.load_all()always returns the parsed data itself as a list containing one or more lists of dictionaries:Iterating over the result will always give lists as items, and obviously
.get()won't work on them.Personally, I'd flatten the list and iterate over all dictionaries, whether they come in one or several YAML documents. I can't really see the advantage of one or the other structure (maybe I'm missing something, though). I normally provide YAML data in one large document, but there may be data sources that use the multi-document approach. It would be sensible to be able to handle both.
@kkthxbye-code commented on GitHub (Nov 23, 2022):
Just for clarity, the following is the format currently supported:
@peteeckel commented on GitHub (Nov 23, 2022):
Hm. That is probably not what anyone would expect when YAML is mentioned. It definitely is a very special case that does not make too much sense to me.
What I would expect in the first place is a list in one document. That's what most tools using YAML export eject, and it's the canonical output when you give yaml.dump with a datastructure roughly equivalent to a CSV:
results in
The multi-document approach is nothing I see very often in everyday YAML practice. It doesn't hurt to support it, but I obviously didn't even have the idea of trying it. The currently supported notation does, however definitely make more sense than the multi-document form of one list element per document I tried.
Anyway, I'd still go for a list of dictionaries.
@peteeckel commented on GitHub (Nov 23, 2022):
I have a suggestion for a minimal code change that would support at least the three variants discussed here:
The above code can handle three variants of YAML structure:
Update: It was still possible to input valid YAML that caused Exceptions. The code now makes sure that what is returned is a list of dictionaries.
@peteeckel commented on GitHub (Nov 23, 2022):
What is feeding my argument as well is that the JSON importer accepts the following data structure:
This is the exact equivalent of the YAML structure
So it really makes sense to support it in addition to the multi-document variant that js supported at the moment. Given the fact that it is currently possible to submit valid YAML structures that cause exceptions trying to parse them
_clean_yaml()needs to be revised anyway.@kkthxbye-code commented on GitHub (Nov 24, 2022):
While I don't have strong feelings either way, if you use
yaml.dump_all()it matches the output netbox expects. Which makes sense as we loadusing yaml.load_all(). Personally I think it would be best to specify one format, but I don't have any good arguments for or against.Whatever we decide, we should at least support the format that the DeviceType export function exports in, which is currently multiple yaml documents via. dump_all(), so it needs to be changed if we change the format.
@peteeckel commented on GitHub (Nov 24, 2022):
I made an experiment, just to check my own perception (sometimes you live in a strange bubble), and asked a few colleagues what they think of first when they should represent the data structure of multiple records with specific fields as a YAML structure. All of them came up with the 'list of dictionaries' idea, some of them didn't even know that the multi-document approach exists (I did, but it was stored away in some very remote attic of my brain).
What's the rationale behind using
dump_all()? Is there any use case that makesdump()impractical or less obvious? I'm trying to figure out the advantage of using what I feel is a slightly exotic format.Anyway, IMHO it makes sense to be tolerant in what you accept and strict in what you emit - so I'd really prefer the approach outlined above of accepting any structure that represents the record structure that needs to be imported.
By the way, I tried to import
(with and without the comma between the dictionaries) as JSON input - both were not accepted. So in JSON the only valid form structure is what would, when parsed as YAML (which is possible, JSON usually is valid YAML as well) be represented as
@jeremystretch commented on GitHub (Nov 28, 2022):
This is the same way the device type YAML import has worked for years.
@peteeckel commented on GitHub (Nov 28, 2022):
Never mind - I can adjust to that, my point is just that IMHO it is a quite unusual variant of YAML.
The main point of this issue still remains - getting the input format wrong while providing a valid YAML structure should not cause an exception later in the process, but a
ValidationErrorwhile cleaning the input.The current code only checks for YAML validity (a test that all of my input variants passed) in
_clean_yamland then an exception is raised later when the parsed data structure does not match the implicitly expected format.