On 06/02/2013 03:43 PM, Amos Jeffries wrote: > 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.
Currently the user can not add a directive_A2 inside such file. It will be treated as options of the directive_A. But this is a desired behaviour. Imagine the case you have a big file with acl values, eg hostnames. You do not want to have them in one line. The code, handles all tokens in a file given from "file:/foo" as tokens exist in the same line. For example: http_port file:"/path/to/port_8082.conf" disable-pmtu-discovery=always /path/to/port_8082.conf: 8082 ssl-bump generate-host-certificates=on options=NO_TLSv1_2 > > 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. The include directive is a different think. > > Amos >
