I also opened CASSANDRA-17548 for removing the flag and extending the in-jvm framework functionality, as we spoke, as a placeholder to flag we need to work on better solution
On Mon, 11 Apr 2022 at 18:24, Ekaterina Dimitrova <e.dimitr...@gmail.com> wrote: > Patch submitted for review in CASSANDRA-17532 > > On Tue, 5 Apr 2022 at 8:56, Ekaterina Dimitrova <e.dimitr...@gmail.com> > wrote: > >> >> Thank you both! >> >> As my understanding is that it will be more involved to deal with the per >> version config loading and that is the reason it was not done yet but a >> flag was added, I suggest we opt in for the flag and revise again this >> after the release. Let’s add it to the other topics we discussed around the >> in-jvm framework. >> >> Two points to support my suggestion: >> - I just checked and there were no new in-jvm upgrade tests added since >> February, before that October and July last year in trunk. 9 months nothing >> added in the older branches. So it seems as a rare thing and just one flag >> to add to the test when writing it. >> - on trunk the old config (pre-15234) is read into the new types so by >> testing with it we actually exercise the whole path now - backward >> compatibility + load into the new types. >> >> Let me know if you have any concerns or questions. >> I will leave the discussion open until tomorrow and in case there are no >> concerns - I will assume lazy consensus and open a ticket to flip the flag. >> >> On Mon, 4 Apr 2022 at 13:41, David Capwell <dcapw...@apple.com> wrote: >> >>> I am cool with checking by default and disabling for tests that need it, >>> it may also make more sense to add an allow list so tests can explicitly >>> say which configs to ignore (though this sounds painful to implement). >>> >>> >>> On Apr 4, 2022, at 9:11 AM, Jon Meredith <jmeredit...@gmail.com> wrote: >>> >>> I think checking the validation rules as part of testing is important, >>> but making that a per-test concern for handling it may be frustrating. >>> >>> What do you think about extending in-JVM with a method that provides the >>> previous config and version and expects the Instance implementation to >>> upgrade it, and then leave the validation on? That would provide some >>> documentation of how deprecated/removed options are expected to be migrated. >>> >>> It might also be worthwhile extending the upgrade tests so that we can >>> also specify new configuration in addition to migrated configurated as the >>> instance is upgraded. >>> >>> On Mon, Apr 4, 2022 at 8:43 AM Ekaterina Dimitrova < >>> e.dimitr...@gmail.com> wrote: >>> >>>> Hi everyone, >>>> In In-jvm tests there is a flag Constants.KEY_DTEST_API_CONFIG_CHECK to >>>> opt-in/out from config checks done in YamlConfigurationLoader#check(). >>>> The upgrade tests are currently set to ignore that check in order to >>>> work around dealing with new properties in newer versions. What does this >>>> mean? My understanding is that the lowest version from which we start an >>>> upgrade test will load Config and use it for all versions. This means that >>>> our checks will fail because in newer versions we set new config in >>>> InstanceConfig that doesn't exist in the old version Config. If we opt in, >>>> the tests will start failing because we need to remove parameters. >>>> >>>> I suggest we opt in by default to the checks so people consciously add >>>> their config. What do I mean? Currently with the new types and names in >>>> trunk, we exercise the backward compatibility and we set the old config >>>> names and values that work with the previous versions and exercise the >>>> backward compatibility but If I add a new name to set for config, this will >>>> just be ignored silently and default Config is used. Test might even >>>> pass...This is risky. >>>> This was documented but I think the right course of action is to opt in >>>> for checks and people ignore the checks in upgrade tests when they are sure >>>> they add the right config and no typos, etc and they understand the >>>> implications. The situation since that check was added has changed and if >>>> we keep on adding more tests, I think this is important so we are sure we >>>> test properly. >>>> >>>> Please let me know if I am wrong in my understanding and what you >>>> think. >>>> >>>> Best regards, >>>> Ekaterina >>>> >>> >>>
