> Sure, a new checkstyle rule was added to address this case for production and > test classes.
Glad to hear =) > On Feb 9, 2023, at 10:45 AM, Maxim Muzafarov <mmu...@apache.org> wrote: > > David, > >> Should” we detect this was violated and fail the build? > > You are asking a good question! Sure, a new checkstyle rule was added > to address this case for production and test classes. > > On Thu, 9 Feb 2023 at 19:40, David Capwell <dcapw...@apple.com> wrote: >> >> All properties meant to be used only for tests would have a prefix like >> "cassandra.test.name.of.property" and production properties would be >> "cassandra.xyx". Once this is done, we can filter them out in vtable so >> there would not be any test-related properties in production. Test >> properties should be visible only when developing / testing Cassandra, in my >> opinion. >> >> >> Good point, I wouldn’t want to expose properties meant to break C* for >> testing… that implies to users we should be using it! >> >> I understand that there is a lot of legacy in place and we can not rename >> properties just like that for people. >> >> >> We could always look to do things like we did in Config (which you call >> out), add a way to “migrate” to the new naming/format. I am not sure if >> there is enough configs to justify this, but with 5.0 happening it may be a >> good time to think about normalizing these. >> >> We are trying to clean up the source code around the direct use of system >> properties and make this use more manageable and transparent. >> >> >> I didn’t review, but one question I have is inline with "I would like to >> describe the ideal state / end goal”, but from the point of view of >> maintaining things…. If someone adds a new system property “should” they use >> this enum? “Should” we detect this was violated and fail the build? >> >> If we migrate now, nothing stops us from adding new, causing someone else to >> be forced to migrate after…. Its one of the issues with the current enum, >> once it was created authors didn’t add to it always causing patches like >> this to try to migrate… >> >> I am not trying to block your patch, if you don’t deal with this I am +0… >> just saying that maintaince is something we must think about >> >> On Feb 9, 2023, at 6:37 AM, Miklosovic, Stefan >> <stefan.mikloso...@netapp.com> wrote: >> >> Hi Maxim, >> >> I would like to describe the ideal state / end goal, from my perspective. >> >> All properties meant to be used only for tests would have a prefix like >> "cassandra.test.name.of.property" and production properties would be >> "cassandra.xyx". Once this is done, we can filter them out in vtable so >> there would not be any test-related properties in production. Test >> properties should be visible only when developing / testing Cassandra, in my >> opinion. >> >> All other system properties should also have some consistent naming in place. >> >> I understand that there is a lot of legacy in place and we can not rename >> properties just like that for people. >> >> The approach I like is what was done to properties in cassandra.yaml. There >> is @Replaces annotation put on properties in Config which enables users to >> still use the old names. >> >> I can imagine that something like this would used here. If an old name is >> specified, it would internally translate to a new name and only new names >> would be returned by vtable. There might be also a column for old names so >> people would know what new property the old one translates to and we should >> also emit warning for users that the system properties they are using are in >> the old format and they should move to the new ones. >> >> Anyway, I am glad this is happening and we are making progress. It will be >> also way easier to dump all properties to the website when everything is >> centralized at once place. >> >> Regards >> >> ________________________________________ >> From: Maxim Muzafarov <mmu...@apache.org> >> Sent: Wednesday, February 8, 2023 19:48 >> To: dev@cassandra.apache.org >> Subject: [DISCUSS] Moving system property names to the >> CassandraRelevantProperties >> >> NetApp Security WARNING: This is an external email. Do not click links or >> open attachments unless you recognize the sender and know the content is >> safe. >> >> >> >> >> Hello everyone, >> >> >> We are trying to clean up the source code around the direct use of >> system properties and make this use more manageable and transparent. >> To achieve this, I have prepared a patch that moves all system >> property names to the CassandraRelevantProperties, which in turn makes >> some of the properties visible to a user through the >> SystemPropertiesTable virtual table. >> >> The patch has passed a few rounds of review, but we still need another >> pair of eyes to make sure we are not missing anything valuable. >> Please, take a look at the patch. >> >> You can find all the changes here: >> https://issues.apache.org/jira/browse/CASSANDRA-17797 >> >> >> I'd also like to share the names of the properties that will appear in >> the SystemPropertiesTable, the appearance of which is related to the >> public API changes we agreed to discuss on the dev list. >> >> >> The public API changes >> >> Newly production system properties added: >> >> io.netty.eventLoopThreads >> io.netty.transport.estimateSizeOnSubmit >> java.security.auth.login.config >> javax.rmi.ssl.client.enabledCipherSuites >> javax.rmi.ssl.client.enabledProtocols >> ssl.enable >> log4j2.disable.jmx >> log4j2.shutdownHookEnabled >> logback.configurationFile >> >> Newly added and used for tests only: >> >> invalid-legacy-sstable-root >> legacy-sstable-root >> org.apache.cassandra.tools.UtilALLOW_TOOL_REINIT_FOR_TEST >> org.caffinitas.ohc.segmentCount >> suitename >> sun.stderr.encoding >> sun.stdout.encoding >> test.bbfailhelper.enabled >> write_survey >> >>
