Upgrading pihole check to support v6+ of pihole #2831
Conversation
monganai
changed the title
|
This pull request has not been updated for more than 21 days. If there are no updates to this PR within 7 days, it will be closed. If you'd like to re-open this PR after it's been closed, you can start from the latest master branch or pull the latest changes into your branch and create a new pull request. |
sarah-witt
left a comment
sarah-witt
left a comment
There was a problem hiding this comment.
Thank you so much for the PR!
| - name: legacy_check | ||
| required: true | ||
| description: Set this to true for pihole v5 support | ||
| value: | ||
| example: false | ||
| type: boolean |
There was a problem hiding this comment.
| - name: legacy_check | |
| required: true | |
| description: Set this to true for pihole v5 support | |
| value: | |
| example: false | |
| type: boolean | |
| - name: legacy_check | |
| required: false | |
| enabled: true | |
| description: Set this to true for pihole v5 support | |
| value: | |
| example: false | |
| type: boolean |
Can we make this enabled by default, but not required so we don't break existing customers?
There was a problem hiding this comment.
Sorry I was not clear enough on this comment! Could you change this back to be false here, but then set default: true? I left a comment on where to add the default in the check code.
sarah-witt
left a comment
sarah-witt
left a comment
There was a problem hiding this comment.
Thanks so much for the update! I was not as clear as I should have been with the configuration behavior, but once this is fixed we can merge 🚀
| if not self.host: # Check if a host parameter exists in conf.yaml | ||
| raise ConfigurationError('Error, please fix pihole.d/conf.yaml, host parameter is required') | ||
| self.web_password = self.instance.get('web_password') | ||
| self.v5_pihole = self.instance.get('legacy_check') |
There was a problem hiding this comment.
| self.v5_pihole = self.instance.get('legacy_check') | |
| self.v5_pihole = self.instance.get('legacy_check', True) |
| - name: legacy_check | ||
| required: true | ||
| description: Set this to true for pihole v5 support | ||
| value: | ||
| example: false | ||
| type: boolean |
There was a problem hiding this comment.
Sorry I was not clear enough on this comment! Could you change this back to be false here, but then set default: true? I left a comment on where to add the default in the check code.
What does this PR do?
Upgrades the pihole check to support version 6
Motivation
The check has been outdated for some time with the upgrade to v6
Review checklist
no-changeloglabel attachedAdditional Notes
Anything else we should know when reviewing?