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

Remove invalid PHP config file and change alert message for db_queries.js #2113

Merged
merged 1 commit into from Feb 12, 2022

Conversation

rdwebdesign
Copy link
Member

@rdwebdesign rdwebdesign commented Feb 8, 2022

By submitting this pull request, I confirm the following:

  • I have read and understood the contributors guide, as well as this entire template.
  • I have made only one major change in my proposed changes.
  • I have commented my proposed changes within the code.
  • I have tested my proposed changes.
  • I am willing to help maintain this change if there are issues with it later.
  • I give this submission freely and claim no ownership.
  • It is compatible with the EUPL 1.2 license
  • I have squashed any insignificant commits. (git rebase)
  • I have Signed Off all commits. (git commit --signoff)

What does this PR aim to accomplish?:

Remove unused .user.php.ini file.
Change the message shown in db_queries page, when an unexpected error occurs.

How does this PR accomplish the above?:

Deleting the file and adding a new message.

What documentation changes (if any) are needed to support this PR?:

None.

dschaper
dschaper previously approved these changes Feb 8, 2022
@yubiuser
Copy link
Member

yubiuser commented Feb 8, 2022

We need to change the docs here: https://docs.pi-hole.net/main/faq/#while-loading-data-from-the-long-term-database-you-encountered-an-error

I wonder if we should decrease the default memory limit. We still support RPi 1, which has only 256 MB. The whole system might freeze if everything is taken by PHP.

@rdwebdesign
Copy link
Member Author

I wonder if we should decrease the default memory limit. We still support RPi 1, which has only 256 MB. The whole system might freeze if everything is taken by PHP.

As far as I know, Raspberry Pis version 1 with 256 MB were discontinued a long time ago (from wikipedia).

Raspberry Pi1B (released in 2012) 256 MB  => Discontinued 
Raspberry Pi1B (released in 2012) 512 MB  => Discontinued 
Raspberry Pi1A (released in 2013) 256 MB  => No ethernet and no Wifi (probably not used for pi-hole)

Raspberry Pi1A+ (released in 2014) 512 MB  => No ethernet and no Wifi (probably not used for pi-hole) 
Raspberry Pi1B+ (released in 2014) 512 MB  => Available

@rdwebdesign
Copy link
Member Author

Do you think the warning message should say something like that?

Check the server's log files (/var/log/lighttpd/error.log) for details.
You may need to increase PHP memory limit in .user.ini file.

Or should we point to the docs page?

Check the server's log files (/var/log/lighttpd/error.log) for details.
You can find more info in pi-hole's FAQ.

@yubiuser
Copy link
Member

yubiuser commented Feb 8, 2022

What happens if user change the limit and perform a Pi-hole update? Will the change be overwritten?

@rdwebdesign rdwebdesign changed the title Rename PHP config file Remove invalid PHP config file and change alert message for db_queries.js Feb 11, 2022
and change alert message for db_queries.js error.

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@yubiuser
Copy link
Member

The goal of this PR changed during the development. Due to merging #2114 the amount of memory Pi-hole uses when retrieving a huge amount of data from the long-term database is dramatically reduced. Therefore there is no reason to increase the memory by default. user.ini wasn't used in the last 5 years since it has been created - there is no reason to keep it.

@yubiuser yubiuser merged commit f4c765a into devel Feb 12, 2022
@yubiuser yubiuser deleted the fix_php_userini branch February 12, 2022 08:22
@pralor-bot
Copy link

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/pi-hole-ftl-v5-14-web-v5-11-and-core-v5-9-released/53529/1

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

4 participants