Re: [PATCΗ] Quoted values in squid.conf

On 06/02/2013 06:43 AM, Amos Jeffries wrote:
> On 1/06/2013 5:14 a.m., Alex Rousskov wrote:
>> 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?

What is? I see no ambiguity. The quoted string in the dynamic_parameters
example is a text string containing a file name. It is just a function
parameter; nothing special is going on here. One could also write:

    dynamic_parameters(config.txt)

If config.txt is a valid token per squid.conf syntax rules. I used
quotes for clarity, to avoid discussing wither '.' or '/' can appear in
unquoted squid.conf tokens.

Please note that, as discussed further below, the following would be
even better than the "local file" access scheme implied above:

    dynamic_parameters("file:/usr/local/etc/config.txt")

>> 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:
>
> 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"

While you may require special directives and special option names to
mark dynamically updated files, that is not the only way to do it, and
is not a very elegant way to do it. From configuration point of view,
supporting a generic mechanism like dynamic_parameters() would be
overall better IMO. Clearly, not all directives will be able to load
parameters dynamically, but that is OK.

    acl foo dstdomain parameters(/somewhere2)
    acl bar dstdomain dynamic_parameters(/somewhere3, check_freq=...)

> The syntax we are deciding is specifically the syntax for the delimiter
> between access method and location.

Not exactly, "file:" only works OK for local locations. file:/somewhere
does not work well for non-local files:

    file:http://somehost/somewhere

On the other hand,

    file(http://somehost/somewhere)

is fine. Hopefully, we can come up with a better name than "file" for
that function, but that is a separate issue.

> 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).

I suggest that we decide on the syntax first and then adjust
implementation, if needed. In my email, I did not really care which C++
method(s) handle file:foo or parameters("file:foo"). I am sure we can
implement this correctly. The difficult part is agreeing on the best syntax.

> 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.

Agreed, but file: works poorly with URIs because they have their own
string: prefix.

>> 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.

Yes. Tying access method and location usually works well. That is why we
have URLs.

Note that "file:" is access method (read a local file), but using
"file:" as the primary marker means that we will need to support
different markers if we were to add other access methods in the future
(e.g., fetch an HTTP resource). And since quite a few squid.conf tokens
start with string: prefix, adding more similar markers will cause more
clashes. This is one more reason to prefer function-like style -- fewer
old tokens will clash with it.

> Which supports the point that these ":" separator
> is the best one to use for simplicity, parser re-use and familiarity for
> the users.

Yes. However, "file:" is a bad choice to identify cases where we need to
load something from a file. In other words:

    acl mine dst parameters("file:/usr/local/etc/my.acls")

is fine, but

    acl mine dst file:/usr/local/etc/my.acls

creates several problems discussed in this thread.

> I say we go with "file:" for now and leave the design of context
> extensions to a later point when such extensions are needed.

If we use file: as a marker now, we will block ourselves from good ways
to support features already anticipated by others. Should not we try to
think one step ahead when it comes to long-lasting configuration syntax
decisions? After all, the reason we are struggling now is because we did
not think of that when the current "filename" syntax was originally
introduced. Let's not step on the same rake twice!

Cheers,

Alex.
Received on Mon Jun 03 2013 - 19:04:50 MDT