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

If exists, import a config file to allow for overriding script variables. #20

Merged
merged 3 commits into from Jun 20, 2015

Conversation

rmceoin
Copy link
Contributor

@rmceoin rmceoin commented Jun 19, 2015

To allow for minor variations of use of the script, parsing a config file could update some of the data locations.

In my case my /etc/pihole/pihole.conf looks like this:

origin=/var/run/pihole
adList=/etc/dnsmasq.d/adList

This would make it so that I wouldn't need to modify future versions of the script. I could also create my own "sources" list and possibly not need to tweak the script.

@jacobsalmela
Copy link
Contributor

I like everything you have done. Have you found that the local-ttl setting improves performance?

Also, it may seem minor, but I typically prefer the double bracket notation for if statements. If you make that change, I will merge your request.

@rmceoin
Copy link
Contributor Author

rmceoin commented Jun 20, 2015

What I did to check the local-ttl was to wireshark and look at what's on the wire. Without the local-ttl queries that ended up using adList had a TTL of 0. And that each time I did a nslookup on a test name it would issue a DNS query to the server. Whereas if it had a TTL greater than 0, the client would issue a DNS on the first nslookup but additional nslookups would not issue a query. That clearly means things should be faster.

I'll check out the double bracket notation.

@jacobsalmela
Copy link
Contributor

Cool. So I'm guessing that is 300 seconds? Is that just an arbitrary number or did you try out different values?

jacobsalmela added a commit that referenced this pull request Jun 20, 2015
If exists, import a config file to allow for overriding script variables.
@jacobsalmela jacobsalmela merged commit 019e122 into pi-hole:master Jun 20, 2015
@jacobsalmela
Copy link
Contributor

Looks great. Let me know what you find out about the value to set.

@rmceoin
Copy link
Contributor Author

rmceoin commented Jun 20, 2015

Yea, it's 300 seconds. I randomly picked a value to be honest. 5 minutes seems reasonable.

Thank you for the merge. I'm trying to gear up for production use of the script/concept at work.

@jacobsalmela
Copy link
Contributor

No problem. It's a great addition. I would love to hear how you get it going at work and the response.

Thanks!

jacobsalmela added a commit that referenced this pull request Jun 21, 2015
You can add your own config file to permanently set variables used in
the gravity script.  If the file exists, gravity.sh will detect it and
apply your custom variables.  This is useful so when there is an update
to the gravity script, you do not need to adjust the variables every
time.
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