[ 
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

Reply via email to