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 CACHE_SIZE to setupVars #3170

Merged
merged 2 commits into from Sep 17, 2020
Merged

Add CACHE_SIZE to setupVars #3170

merged 2 commits into from Sep 17, 2020

Conversation

DirkJanIT
Copy link
Contributor

By submitting this pull request, I confirm the following:
please fill any appropriate checkboxes, e.g: [X]

  • 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, and have included unit tests where possible.
  • 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)

Please make sure you Sign Off all commits. Pi-hole enforces the DCO.


What does this PR aim to accomplish?:
https://discourse.pi-hole.net/t/add-cache-size-to-setupvars-conf/28667

How does this PR accomplish the above?:
Adds the necessary variable to setupVars.conf.

What documentation changes (if any) are needed to support this PR?:
Document the addition of CACHE_SIZE in setupVars.conf.

Signed-off-by: DoubleOhmSeven <57564379+DoubleOhmSeven@users.noreply.github.com>
Signed-off-by: DoubleOhmSeven <57564379+DoubleOhmSeven@users.noreply.github.com>
@dschaper
Copy link
Member

https://docs.pi-hole.net/ftldns/dns-cache/ will need to be aligned with this PR.

@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/add-cache-size-to-setupvars-conf/28667/56

DL6ER
DL6ER previously approved these changes Jul 19, 2020
@DL6ER
Copy link
Member

DL6ER commented Jul 19, 2020

@dschaper Are you okay with overwriting Travis here? It seems to be dead.

@dschaper
Copy link
Member

No, travis is failed for a reason. Note the branches.

@DL6ER DL6ER changed the base branch from master to development July 19, 2020 21:02
@DL6ER DL6ER dismissed their stale review July 19, 2020 21:02

The base branch was changed.

@DL6ER
Copy link
Member

DL6ER commented Sep 8, 2020

@dschaper Are you still okay with this being merged? If so, please go ahead.

@dschaper
Copy link
Member

dschaper commented Sep 8, 2020

Need the .travis.yml file from master (or development) to be merged in to the PR so travis will check it.

@DL6ER
Copy link
Member

DL6ER commented Sep 17, 2020

The source repository doesn't exist any more

@dschaper
Copy link
Member

If you think it's a good PR then we can override it.

@DL6ER DL6ER merged commit b88510d into pi-hole:development Sep 17, 2020
@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-core-web-v5-2-and-ftl-v5-3-released/40909/1

@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/correct-way-to-change-cache-setting/42699/2

@o5ky
Copy link

o5ky commented Feb 12, 2021

FYI this breaks when you upgrade from a previous older version using the command below. It inserts the variable instead of the variable value.
/etc/.pihole/automated\ install/basic-install.sh --repair --unattended

cache-size=@CACHE_SIZE@

FTL failed to start due to error at line 32 of /etc/dnsmasq.d/01-pihole.conf

@dschaper
Copy link
Member

Is there a reason you didn't want to use the built in upgrade process of pihole -up instead of an undocumented and unsupported command line?

@o5ky
Copy link

o5ky commented Feb 12, 2021

I mean that is how to repair and it's a supplied script, it's sometimes required when the system has failed and requires the repair switch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants