Skip to content

Conversation

@yubiuser
Copy link
Member

What does this PR aim to accomplish?:

  1. Removes the json output functionality. Pi-hole v6 has a new API which will respond with JSON by default. Instead of misusing PADD for some kind of stats output in JSON, users should directly interact with the API
  2. Fix the internal update function. It was not ported to v6 so far, esp. it rely on the presence of /etc/pihole/versions which does not exist when PADD is run remotely.

By submitting this pull request, I confirm the following:

  1. I have read and understood the contributors guide, as well as this entire template. I understand which branch to base my commits and Pull Requests against.
  2. I have commented my proposed changes within the code and I have tested my changes.
  3. I am willing to help maintain this change if there are issues with it later.
  4. It is compatible with the EUPL 1.2 license
  5. I have squashed any insignificant commits. (git rebase)
  6. I have checked that another pull request for this purpose does not exist.
  7. I have considered, and confirmed that this submission will be valuable to others.
  8. I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  9. I give this submission freely, and claim no ownership to its content.

  • I have read the above and my PR is ready for review. Check this box to confirm

@yubiuser yubiuser requested a review from a team February 26, 2025 12:53
@github-actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

DL6ER
DL6ER previously approved these changes Mar 1, 2025
Copy link
Member

@DL6ER DL6ER left a comment

Choose a reason for hiding this comment

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

It is unclear to me how I should test the update mechanism so I mostly base this approval on reading the code and doing only very simple tests like ./padd.sh -v and ./padd.sh -u

Establishing connection with FTL...
Authentication successful.

PADD version is v4.0.0 (Latest: v4.0.0)


Session successfully deleted.
Establishing connection with FTL...
Authentication successful.
[✓] You are already using the latest PADD version v4.0.0


Session successfully deleted.

Maybe a follow-up PR can reduce the verbosity concerning API authentication here.

@github-actions
Copy link

github-actions bot commented Mar 4, 2025

Conflicts have been resolved.

@yubiuser yubiuser changed the title Drop JSON output and fix udpate command Drop JSON output and allow udpate command without authentication Mar 6, 2025
@yubiuser
Copy link
Member Author

yubiuser commented Mar 6, 2025

I added a commit that allows to perform the update without login. I think there is no need to authenticate just to check/perform the update of this independent script. I also allowed to update in case PADD is running on docker (which was the only reason we used data from FTL and needed to login) - it will be overwritten on the next container creation, but why not?


Testing the updater is simple: decrease the version number < 4.00 and run padd -u

@yubiuser yubiuser changed the title Drop JSON output and allow udpate command without authentication Drop JSON output and allow update command without authentication Mar 28, 2025
Copy link
Member

@DL6ER DL6ER left a comment

Choose a reason for hiding this comment

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

Tested, works

@DL6ER DL6ER merged commit 590ceeb into development Mar 28, 2025
5 checks passed
@DL6ER DL6ER deleted the json branch March 28, 2025 16:07
@yubiuser yubiuser linked an issue Mar 29, 2025 that may be closed by this pull request
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.

In the "JSON" option, provide Last Gravity Update

3 participants