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

Added more startup_scripts and initializers examples. #143

Merged
merged 7 commits into from
Oct 12, 2019

Conversation

axarriola
Copy link

Summary

Added more startup_scripts and initializers examples for further Netbox objects.

Files

initializers/aggregates.yml
initializers/cluster_types.yml
initializers/clusters.yml
initializers/dcim_interfaces.yml
initializers/ip_addresses.yml
initializers/prefixes.yml
initializers/rirs.yml
initializers/tenants.yml
initializers/virtual_machines.yml
initializers/virtualization_interfaces.yml
initializers/vlan_groups.yml
initializers/vlans.yml
initializers/vrfs.yml
startup_scripts/104_tenant_groups.py
startup_scripts/105_tenants.py
startup_scripts/120_cluster_types.py
startup_scripts/125_rirs.py
startup_scripts/130_aggregates.py
startup_scripts/150_clusters.py
startup_scripts/155_vrfs.py
startup_scripts/160_prefix_vlan_roles.py
startup_scripts/170_vlan_groups.py
startup_scripts/180_vlans.py
startup_scripts/190_prefixes.py
startup_scripts/200_virtual_machines.py
startup_scripts/210_virtualization_interfaces.py
startup_scripts/215_dcim_interfaces.py
startup_scripts/220_ip_addresses.py

Copy link
Collaborator

@cimnine cimnine left a comment

Choose a reason for hiding this comment

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

Thank you a lot for this PR. It must have been hard work figuring all this out. I feel a little bit bad for adding so many remarks. Could you please see through them and either fix them or otherwise add a short note if you don't agree or if what I ask for is too much work.

Again, thank you very much for sharing your work with the community.

@cimnine cimnine added the enhancement The issue describes an enhancement that we would like to implement in the future. label Jul 10, 2019
@cimnine
Copy link
Collaborator

cimnine commented Jul 29, 2019

@axarriola Are you still interested in getting this PR merged? Have you had the chance to look at my comments?

@axarriola
Copy link
Author

Hi @cimnine , of course, I've already fixed over half of the comments. I've just been swamped with work in the last weeks. Hopefully I will get some time this week to finish everything. Is that ok?

@cimnine
Copy link
Collaborator

cimnine commented Jul 29, 2019

Cool! Don't worry and take your time.

@axarriola
Copy link
Author

@cimnine Just wanted to let you know that this is still on my to-do list, just haven't found the time, but I'll get to it.

@cimnine
Copy link
Collaborator

cimnine commented Oct 10, 2019

FYI: Don't worry about the failing build. That's something I'm working on in parallel.

@axarriola
Copy link
Author

I'm still missing a few comments, working on those at the moment.

@axarriola
Copy link
Author

axarriola commented Oct 10, 2019

@cimnine Now all the remarks have been taken care of! Due to some events today I had some free time. Kindly let me know if there is anything else

Copy link
Collaborator

@cimnine cimnine left a comment

Choose a reason for hiding this comment

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

I'm very excited about these initializers! But I have some more comments before I'm confident to merge those.

Please have a look on the remarks – and feel free to disagree where you believe this is due. For all that matters, we can turn a blind eye to all the comments on the yaml files. (But please don't resolve them when they're not actually resolved.)

@axarriola
Copy link
Author

I submitted another commit. To my knowledge I changed all the remarks to the .yaml files, I went one by one, let me know which ones weren't updated.

Copy link
Collaborator

@cimnine cimnine left a comment

Choose a reason for hiding this comment

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

I was a little bit disappointed when I commented in all the yamls and Netbox was not able to start. Nevertheless, here are some more suggestions and fixes.

Please make sure, that – when enabling all the code in the yaml files, and with a clean database – a docker-compose up succeeds. (You can clean the database with docker-compose down -v.)

I've also made some changes to the master branch regarding the initializers, which you might want to merge into your branch. (Or a rebase + force-push is fine with me as well.)

# vlan: vlan2
# is_pool: true
# vrf: vrf2
# tenant: tenant2
Copy link
Collaborator

@cimnine cimnine Oct 11, 2019

Choose a reason for hiding this comment

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

The tenant key is defined twice. Duplicate keys are not allowed by the Netbox code.

Suggested change
# tenant: tenant2

Copy link
Author

Choose a reason for hiding this comment

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

Fixed.

@axarriola
Copy link
Author

I submitted all the changes and tested them before in a new netbox-docker on a fresh VM.

Copy link
Collaborator

@cimnine cimnine left a comment

Choose a reason for hiding this comment

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

🎉 So cool! I will merge it right away :)

@cimnine cimnine merged commit 992a8f1 into netbox-community:master Oct 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement The issue describes an enhancement that we would like to implement in the future.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants