On 1/06/2013 5:14 a.m., Alex Rousskov wrote:
On 05/31/2013 10:05 AM, Tsantilas Christos wrote:
On 05/27/2013 07:12 PM, Alex Rousskov wrote:
On 05/27/2013 04:54 AM, Tsantilas Christos wrote:
On 05/26/2013 10:05 PM, Kinkie wrote:
The only thing I would like to see differently implemented is the
syntax used to include files:
file(path) would be IMO easier to understand and less prone to
confusion than the proposed syntax.
OK.
But imagine in the future also the following syntax:
file:/path/file
system:/usr/local/squid/bin/my-squid-conf (to read from an executable
stdout configuration options)
http://hostname/cfgfile (to get from web page configuration)
All the above can be implemented in the future...
Sure, I agree.
file(/path/file)
system(/some/executable)
http_get(http://hostname/file)
Well, this is not a bad scheme :-)
Just the file:/path/to/file a little easier to implement. But not
something important...
I agree that file() is a good alternative. Amos (and others), do you
have a preference between
file:"/path/file"
and
file("/path/file")
syntax?
About these two schemes I have a different suggestion.
Some opensource packages using the "file:/" or "db:/" for dynamic data.
I do not know if it make sense for squid but I am suggesting the
following scheme
1) Use the file("path") syntax for configuration file parsing
2) preserve the file:/ or db:/ etc for dynamic data. For example acls
values stored in bdb or sql server.
Stepping back a little, I think there are two big problems with the "URL
scheme-like" approaches (file:, db:, etc.):
1) They combine the method of access (local file, HTTP, database query,
etc.) with the meaning of the received information (configuration values
to use now or configuration values to interpret at the time of directive
use). With function-like style, this is not a problem:
# static configurations to use now:
parameters("/usr/local/etc/config.txt")
parameters("http://example.com/config.txt";)
parameters("db:config_table")
# dynamically updated configuration:
dynamic_parameters("/usr/local/etc/config.txt")
The above is rather ambiguous, is that a text string or a filename? this
is the problem we seek to resolve with "file:" prefix.
dynamic_parameters("http://example.com/config.txt";)
dynamic_parameters("db:config_table")
I think you have stepped back too far on this point. The interpretation
is imposed by the parser context:
include file:/somewhere
acl foo dstdomain file:/somewhere2
directive_with_options parameter=file:/somewhere3
fancy_directive parameters="http://example.com/config.txt";
more_parameters="http://example.com/config.txt method=QUERY"
The syntax we are deciding is specifically the syntax for the delimiter
between access method and location. We seem to be getting side-tracked
by the implementation choice of placing these checks inside NextToken()
due to the first two lines above being representative of how files are
currently used in squid.conf. They are in no way representative of the
future possibilities and should not be limiting the potential usage
anywhere elase in the config (as demonstrated by the latter two of my
lines above, and most of your examples).
Also the access methods we will have here are likely to be quite
restricted, either to direct filesystem methods or network locations -
all of which are URI-representable methods.
2) They make it difficult to add other options/parameters for that
access (e.g., HTTP request method or database reload frequency). With
function-like style, this is not a problem:
parameters("http://example.com/config.txt";, method=QUERY)
dynamic_parameters("db:config_table", reload_frequency=1s)
I notice you are using the db:config_table syntax here for the access
method and location. Which supports the point that these ":" separator
is the best one to use for simplicity, parser re-use and familiarity for
the users.
I say we go with "file:" for now and leave the design of context
extensions to a later point when such extensions are needed.
Back to the code ...
As submitted the presence of specific checks for file:"..." in
NextToken() seems to be a bit restrictive on what can be done with them
and appears to loose the EOL marker for detecting the end of current
directive options. I think nextToken() should just return the
file:whatever token to the requestor and let that directives context
handle whether it becomes a file parsed by the ConfigParser or not.
Exposing a method which allows handlers like include directive to push a
file to be loaded onto the stack of files currently being parsed should
be sufficient for that.
If we allow the config to side-track itself into a sub-file at any point
we could end up with weird/undersirable things like this happening:
squid.conf:
directive_A param file:/foo other_parameters
foo:
param2
new_directive something
directive_A2
and directive_A2 gets the other_parameters instead of directive_A1.
Which is rather far from what we want in most situations.
We have two main use-cases for sub-files right now; the include
directive - where the file is a set of directive lines, and everywhere
else - where the file is a series of options for the current directive
at one per line.
Amos
