[
https://issues.apache.org/jira/browse/HIVE-16642?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16011099#comment-16011099
]
Sushanth Sowmyan commented on HIVE-16642:
-----------------------------------------
[~anishek], thanks for the feedback, that was most useful. TestRule seem to be
exactly what I was looking for and didn't know existed. Now I've gotten rid of
the hacky zzz* naming for these tests, and this works as an addendum to each
individual test. Earlier, I didn't add this to each individual test because
\@After annotation is not useful in that failures inside it don't fail the test
itself, and I didn't want to add instrumentation in each and single test case
to call something after it had run(easy to skip adding this with future
additions). Thus, having a single test after everything, with
\@FixedMethodOrdering was my compromise.
But, with TestRule, it was easy to rewrite it closer to how I wanted it. I
still have one sore point in my current update, in that some tests should not
call this Rule, and I needed a way to skip the rule for particular tests.
However, the closest I got to being able to do that is to instrument a skipList
and then evaluate it inside the rule - I would have preferred something on the
lines of an annotation so that I can skip this additional Rule-check but not
Ignore the test or other Rules that might exist. I tried to see if I could get
the set of annotations from the Description, but that somehow does not pass
along any custom annotations I create, passing along only @Test . If you can
think of a cleaner way to do that, I'm all ears.
In the meanwhile, though, this latest version of the patch should be good to go.
Also, as to your question on why this does not belong to itests/hive-unit , I
think the correct place is in the same place as ReplicationTask, in that it is
intended to be used by those that use Events to verify that they are not
breaking compatibility with ReplicationTask. Thus, this is something that is
potentially usable by code outside of hive, such as in Falcon. Having it be in
a test-jar artifact for the same package that contains ReplicationTask does
sound like the right place to me. However, thinking on that, then it doesn't
make sense for it to be called "Backward" compat, and that naming is confusing,
since it is more of a replv1-compat-checker. So, along with renaming it to a
Rule as per the new changes, I've dropped the word "Backward" to make that a
bit clearer.
> New Events created as part of replv2 potentially break replv1
> -------------------------------------------------------------
>
> Key: HIVE-16642
> URL: https://issues.apache.org/jira/browse/HIVE-16642
> Project: Hive
> Issue Type: Sub-task
> Components: repl
> Reporter: Sushanth Sowmyan
> Assignee: Sushanth Sowmyan
> Attachments: HIVE-16642.1.patch, HIVE-16642.2.patch
>
>
> We have a couple of new events introduced, such as
> \{CREATE,DROP\}\{INDEX,FUNCTION\} since the introduction of replv1, but those
> which do not have a replv1 ReplicationTask associated with them.
> Thus, for users like Falcon, we potentially wind up throwing a
> IllegalStateException if replv1 based HiveDR is running on a cluster with
> these updated events.
> Thus, we should be more graceful when encountering them, returning a
> NoopReplicationTask equivalent that they can make use of, or ignore, for such
> newer events.
> In addition, we should add additional test cases so that we track whether or
> not the creation of these events leads to any backward incompatibility we
> introduce. To this end, if any of the events should change so that we
> introduce a backward incompatibility, we should have these tests fail, and
> alert us to that possibility.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
