-
Notifications
You must be signed in to change notification settings - Fork 6
Add infrahubctl object
command
#328
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
Conversation
Codecov ReportAttention: Patch coverage is @@ Coverage Diff @@
## develop #328 +/- ##
===========================================
+ Coverage 73.05% 73.83% +0.78%
===========================================
Files 92 92
Lines 8202 8486 +284
Branches 1582 1657 +75
===========================================
+ Hits 5992 6266 +274
+ Misses 1797 1787 -10
- Partials 413 433 +20
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing around this that isn't part of this PR the code already looked like this before but if we look at the example from the PR description:
---
apiVersion: infrahub.app/v1
kind: Object
spec:
kind: TestingPerson
data:
- name: Mike Johnson
height: 175
best_friends: # Relationship of cardinality many that referenced existing nodes based on their HFID
- [Jane Smith, Max]
- [Sarah Williams, Charlie]
If TestPerson "Mike Johnson" existed going into this and had other best_friends relationships defined I think that those earlier relationships would be replaced with the ones defined here instead of just ensuring that the ones above also exist.
I'm not certain how I'd want it to work here, just highlighting to raise the issue. I think there are merits to both approaches, just not sure what users would expect to happen. Any thoughts?
d1ff928
to
b74995b
Compare
infrahub_sdk/ctl/object.py
Outdated
for item in file.spec.data: | ||
await file.spec.create_node(client=client, schema=schema, data=item, branch=branch) | ||
for file in files: | ||
await file.process(client=client, branch=branch) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same SOLID comment here as for validate_format
a couple lines above
also, process
seems more like a create
method to me
(infrahub-sdk-py3.12) ➜ bug-hunting-space infrahubctl schema check oneoone.yml
schema 'oneoone.yml' is Valid!
diff:
added:
RandomStuff:
added: {}
changed: {}
removed: {}
changed: {}
removed: {}
(infrahub-sdk-py3.12) ➜ bug-hunting-space infrahubctl object validate objects/server-template.yml
(infrahub-sdk-py3.12) ➜ bug-hunting-space
|
---
apiVersion: infrahub.app/v1
kind: Object
spec:
kind: TemplateComputePhysicalServer
data:
- template_name: Test_pve
interfaces:
kind: TemplateInterfacePhysical
data:
- template_name: eth0
name: eth0
enabled: false
not: valid
description: This interface comes from CLI
l2_mode: tunnel
(infrahub-sdk-py3.12) ➜ bug-hunting-space infrahubctl object load objects/server-template.yml
[10:29:01] INFO Node: Test_pve object.py:217
Error: Object is not valid
Traceback (most recent call last):
File "/Users/Baptiste/Workspaces/Infrahub/infrahub-sdk-python/infrahub_sdk/ctl/utils.py", line 89, in async_wrapper
return await func(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/Baptiste/Workspaces/Infrahub/infrahub-sdk-python/infrahub_sdk/ctl/object.py", line 46, in load
await file.process(client=client, branch=branch)
File "/Users/Baptiste/Workspaces/Infrahub/infrahub-sdk-python/infrahub_sdk/spec/object.py", line 312, in process
await self.spec.process(client=client, branch=branch)
File "/Users/Baptiste/Workspaces/Infrahub/infrahub-sdk-python/infrahub_sdk/spec/object.py", line 88, in process
await self.create_node(client=client, schema=schema, data=item, branch=branch)
File "/Users/Baptiste/Workspaces/Infrahub/infrahub-sdk-python/infrahub_sdk/spec/object.py", line 232, in create_node
await cls.create_related_nodes(
File "/Users/Baptiste/Workspaces/Infrahub/infrahub-sdk-python/infrahub_sdk/spec/object.py", line 273, in create_related_nodes
node = await cls.create_node(
^^^^^^^^^^^^^^^^^^^^^^
File "/Users/Baptiste/Workspaces/Infrahub/infrahub-sdk-python/infrahub_sdk/spec/object.py", line 154, in create_node
raise ValidationError(identifier=schema.kind, message="Object is not valid")
infrahub_sdk.exceptions.ValidationError: Object is not valid
(infrahub-sdk-py3.12) ➜ bug-hunting-space When creating invalid files, it's hard to tell where the issue comes from just by looking at the command output. If possible, it would be helpful to guide users by:
|
In some situation we could have multiple kinds hidden behind a single relationship: Defining the ---
apiVersion: infrahub.app/v1
kind: Object
spec:
kind: TemplateComputePhysicalServer
data:
- template_name: Test_pve
interfaces:
kind: TemplateInterfacePhysical
data:
- template_name: eth0
name: eth0
enabled: false
not: valid
description: This interface comes from CLI
l2_mode: tunnel I believe there's still a workaround by splitting interfaces into multiple files... |
This one has been bothering me as well and this is one of the reason I haven't merged this PR yet there are a few options possible but I'm not sure which one is the best Option 1: each item in the list includes
|
Hmm, this one's tricky. I'd say option 1 is easier for users to understand. Option 2 is a bit more complex (a list within a list), but it's consistent with the rest of the file and could save users some time by not having to repeat the kind each time. So I'd lean toward option 1, but I don't have any strong objections to option 2. |
b74995b
to
3e52900
Compare
Deploying infrahub-sdk-python with
|
Latest commit: |
cd9ee51
|
Status: | ✅ Deploy successful! |
Preview URL: | https://2617037e.infrahub-sdk-python.pages.dev |
Branch Preview URL: | https://dga-20250330-object-cmd.infrahub-sdk-python.pages.dev |
Option 1 has my preference. It is more verbose, but it is very clear and explicit on what is going to happen.
I think it should update the relationship with the related nodes that are declared in the file. If not, we will need to find a way for a user to declare that he wants to remove a related node from an object. AKA we will have to do something very similar to how we allow people to remove things in the schema files. Most of our users don't seem to like that behavior. |
I must be missing something because I can't make it work with the new syntax: ---
apiVersion: infrahub.app/v1
kind: Object
spec:
kind: TemplateComputePhysicalServer
data:
- template_name: Test123
interfaces:
- kind: TemplateInterfacePhysical
data:
template_name: eth0
name: eth0
enabled: false
l2_mode: tunnel infrahub_sdk.exceptions.ValidationError: Object is not valid I updated SDK to the latest version on this branch... |
434a7bd
to
feb5b1e
Compare
Regarding
having said that , I understand your point but not sure what is the best way proceed here |
When I try to add more than one component object I have an error message: ---
apiVersion: infrahub.app/v1
kind: Object
spec:
kind: ComputePhysicalServer
data:
- name: Server1234
status: active
location: rack00
interfaces:
- kind: InterfacePhysical
data:
name: eth0
enabled: false
l2_mode: access
- kind: InterfacePhysical
data:
name: eth1
enabled: false
l2_mode: access (infrahub-sdk-py3.12) ➜ bug-hunting-space infrahubctl object load objects/server.yml
[10:59:55] INFO Node: Server1234 object.py:425
INFO Node: InterfacePhysical : 183623bb-e28a-9ea8-32b3-c51e3dd3f375 object.py:425
['InterfacePhysicalUpsert'] InterfacePhysical - 183623bb-e28a-9ea8-32b3-c51e3dd3f375 cannot be added to relationship, must be of type: ['ComputePhysicalServer', 'DcimDevice', 'VirtualizationVirtualMachine'] at device This works perfectly fine if I only have one row in the (infrahub-sdk-py3.12) ➜ bug-hunting-space infrahubctl object load objects/server.yml
[11:05:35] INFO Node: Server1234 object.py:425
INFO Node: InterfacePhysical : 183623bb-e28a-9ea8-32b3-c51e3dd3f375 object.py:425 |
523d965
to
ef91f25
Compare
e05ef4f
to
cd9ee51
Compare
Thanks @BaptisteGi, this one was also reported by the integration tests, it has been fixed |
Add
infrahub object load
andinfrahub object validate
commands to load objects defined in Yaml format into Infrahub.These commands are meant to be used in an idempotent way and as such they would work better for models with a Human Friendly ID (hfid) defined.
File format
All object files must start with the following format, all other format will be automatically ignored
Each file is intended for one specific top level kind, but one file can include multiple nested objects of any kind.
The kind of the top level object must be defined in
spec/kind
Below is a more detailed example with nested objects and with related objects referenced by their HFIDs