Skip to content

Drop Into<ConfigBuilder> for Config

Or to put it another way, make it so that you cannot

  • make a config
  • make a TorClient out of it
  • retrieve the config from the TorClient
  • make some ad-hoc modification to that config
  • reconfigure your TorClient with the new config (or create a new TorClient from it).

IMO this is part of #285 (closed) and blocking #371 (closed).

I think that the above sequence of events is impossible (or, at least, very difficult) to make correct in the face of future configuration layout changes and the associated compat code. (See IRC scrool below.)

Users who want to do this should be advised to keep the builder (which after #371 (closed) will be called the Config, not the ConfigBuilder, or the JSON or whatever they deserialised it from. Reusing the builder and modifying it and reconfiguring it will work correctly even after config layout changes.

16:24 <+nickm> Diziet: if we decide in the future to expose some configuration 
               values via an API, how do you see that working?  for example, 
               suppose that we want to expose a way to get the cache_dir for a 
               TorClient.
16:25 <+Diziet> I guess we could expose the FooConfigValidated type in tor-foo, 
                and provide a get_wombat method.
16:26 <+Diziet> And accessors to get at the bits from a TorClient, and a nice 
                pub wrapper in arti-client.
16:26 <+Diziet> Is this a thing you think we will want to do a lot ?
16:26 <+eta> I mean I think it makes sense to ask a TorClient what its config is
16:26 <+Diziet> In a format you can feed to another TorClient ?
16:26 <+eta> I think I'd rather keep the validated types private and convert 
             back into a builder though
16:27 <+nickm> eta: That's doable if we retain the `Into<FooConfigBuilder> for 
               FooConfig` implementations, which I think we should
16:27 <+nickm> (modulo renaming)
16:27 <+Diziet> That conversion might involve ad-hoc config field specific 
                code, depending what conversions happen during validation.
16:28 <+Diziet> (I don't think we have very much such code, only at most 
                type-specific code which is fine by me.)
16:28 <+nickm> Also, down the road, do you think it makes sense to have a way 
               to inspect the current value of a field in the builder, in 
               addition to setting it?
16:29 <+nickm> (While working on arti-testing I've wanted to do both of the 
               above, though I'm not sure if I'm right to do so)
16:29 <+eta> I think we definitely want that
16:30 <+Diziet> nickm: The builders are going to be Serialize.
16:30 <+Diziet> So worst case is you use that.
16:31 <+nickm> hm. I guess doing that stuff at the `config` layer is much 
               easier...
16:31 <+Diziet> Yes.
16:32 <+Diziet> I think I would prefer to avoid Rust-API-level 
                backward-conversions.  It makes things more complicated.  
                Especially, we'd have to think what the semantics ought to be 
                after we have changed the config layout.
16:32 <+nickm> by "backward conversions" do you mean the Into<> thing I 
               described above?
16:32 <+Diziet> Yes, broadly.
16:33 <+Diziet> If you ask for the config of a TorClient, it is going to have 
                resolved the config back compat stuff.
16:33 <+Diziet> And the result isn't therefore going to be API-stable any more.
16:33 <+nickm> Hm.  I know we use that in places, though.
16:33 <+nickm> It does make it harder to reconfigure a thing if you can't get 
               its configuration into a builder.
16:33 <+nickm> though maybe you should just keep the builder
16:33 <+Diziet> Eg you might FooConfig.wombat(42) and then turn it into a 
                TorClient, and when you ask for the config back, it wouldn't 
                have a wombat any more, only a marsupial.
16:34 <+Diziet> Yes, I think you should keep the builder (or the json or 
                whatever)
16:34 <+Diziet> Reconfiguring after backconverting the configuration is going 
                to be hazardous too.  Eg, what if you try to backconvert and 
                set the wombat and then reconfigure ?