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

Add netbox_database_maintenance variable #158

Merged
merged 1 commit into from
Feb 2, 2023
Merged

Add netbox_database_maintenance variable #158

merged 1 commit into from
Feb 2, 2023

Conversation

MonsterDruide1
Copy link
Contributor

Some use cases might not be able to access the default postgres database. To make sure that this playbook also works in these scenarios, maintenance_db should be changed by using variables, passed from outside of the role.

The default should still remain on postgres:
https://docs.ansible.com/ansible/latest/collections/community/postgresql/postgresql_db_module.html

tasks/main.yml Show resolved Hide resolved
Copy link
Owner

@lae lae left a comment

Choose a reason for hiding this comment

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

Actually, almost forgot. Please update the README.adoc to include documentation for the new role variable and its usage.

@MonsterDruide1
Copy link
Contributor Author

I have added it ... somewhere, not quite sure where/how you'd want me to document it.

@lae
Copy link
Owner

lae commented Jan 29, 2023

There's a segment under Role Variables that goes over the other database connection parameters. That would be where. Ctrl+F "netbox_database" should get you to it, I think, if you're not sure where.

Since this is one of those "edge case" parameters, it should probably get its own line, similar to the connection age variable, and an explanation of how it's used (e.g. ansible connects to it by default) and why one would change it.

@lae
Copy link
Owner

lae commented Jan 30, 2023

@MonsterDruide1 I think you might have forgot to force push the documentation update, so pinging you just in case. (Also see my last comment if you need it.)

@MonsterDruide1
Copy link
Contributor Author

Sorry, took some time with other stuff - does this documentation match your request?

README.adoc Outdated Show resolved Hide resolved
README.adoc Outdated Show resolved Hide resolved
@lae
Copy link
Owner

lae commented Feb 2, 2023

Yeah, the documentation looks fine apart from the minor things I pointed out.

@lae lae merged commit efca97d into lae:master Feb 2, 2023
@MonsterDruide1 MonsterDruide1 deleted the maintenance-db branch February 3, 2023 10:05
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.

None yet

2 participants