[ https://issues.apache.org/jira/browse/CASSANDRA-19731?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17860887#comment-17860887 ]
Doug Rohrer commented on CASSANDRA-19731: ----------------------------------------- On the surface, this seems like a really nice "quality of life" improvement, and I'm generally +1 on the idea... that said, I'm curious what it means practically. If you look at the history of the {{org.apache.cassandra.exceptions}} package there seem to be quite a few non-backwards/forwards-compatible changes in constructors, message formats, etc. I wonder what happens when one of those classes is shared, and upgrade tests are run. I would assume we'd end up with other weird failures depending on which version was loaded first. Would we have to declare that all classes of the {{exceptions}} package must be backward-and-forward-compatible with previous versions of the exception? Would we have to version exceptions (so, {{RequestFailureReasonV51}} if 5.1 added some new functionality, but still extend the base class?) Or maybe we have a different set of shared classes for upgrade tests vs. non-upgrade tests (which is totally doable I think), at which point maybe this isn't as big of a concern (although I'd want to have the default list of shared packages returned from {{getDefaultLoadSharedFilter}} be version-agnostic, and then maybe a second {{getSingleVersionSharedFilter}} method that could include more packages?) > Consider adding exceptions among shared packages in in-jvm dtest api > -------------------------------------------------------------------- > > Key: CASSANDRA-19731 > URL: https://issues.apache.org/jira/browse/CASSANDRA-19731 > Project: Cassandra > Issue Type: Improvement > Components: Test/dtest/java > Reporter: Stefan Miklosovic > Assignee: Stefan Miklosovic > Priority: Normal > > This does not work > {code} > try (Cluster ignored = build(1) > .withConfig(c -> c.with(Feature.NETWORK, > Feature.NATIVE_PROTOCOL, Feature.GOSSIP) > > .set("some_property_which_throws_when_misconfigured", value)) > .start()) > { > fail("should throw ConfigurationException"); > } > catch (Exception ex) > { > assertEquals(ConfigurationException.class, ex.getClass()); > } > {code} > Because ConfigurationException is not shared so it looks like it doesn't > match. > this is workaround > {code} > Predicate<String> EXTRA = className -> > className.equals(ConfigurationException.class.getName()); > Cluster.Builder builder = build(1); > assertThatThrownBy(() -> builder > > .withSharedClasses(EXTRA.or(builder.getSharedClasses())) > .start()) > .isInstanceOf(ConfigurationException.class); > {code} > But to have it nice and clean like this, it would require us to share all > exceptions, to add them to "DEFAULT_SHARED_PACKAGES". in > "org.apache.cassandra.distributed.shared.InstanceClassLoader". > {code} > assertThatThrownBy(() -> build(1).withConfig(c -> > c.set("some_property_which_throws_when_misconfigured", value)).start()) > .isInstanceOf(ConfigurationException.class); > {code} > We could just put all package there. > This would require release of in-jvm dtest api. -- This message was sent by Atlassian Jira (v8.20.10#820010) --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org