You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by kalyan kumar kalvagadda <kk...@cloudera.com> on 2017/05/25 15:54:05 UTC

Review Request 59576: SENTRY-1785 Fix TestKrbConnectionTimeout test

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59576/
-----------------------------------------------------------

Review request for sentry, Alexander Kolbasov, Brian Towles, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.


Bugs: SENTRY-1785
    https://issues.apache.org/jira/browse/SENTRY-1785


Repository: sentry


Description
-------

There are couple of issues here
1. When Assume.assumeTrue fails the test is skipped but some times it is not skipped and the test fails.
2. setup(0 method in TestKrbConnectionTimeout calls setup again, which is reclusive call and there will be stack overflow.

Patch submitted has three changes
1. Updated the requires system property "sentry.hive.test.ticket.timeout" to true.
2. rename the BeforeClass annotated method of TestKrbConnectionTimeout class to avoid recursive call.
3. Invoked setup method to beginning of BeforeClass annotated method of TestKrbConnectionTimeout.


Diffs
-----

  sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestKrbConnectionTimeout.java b62a83f 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java dd37e7e 


Diff: https://reviews.apache.org/r/59576/diff/1/


Testing
-------


Thanks,

kalyan kumar kalvagadda

Re: Review Request 59576: SENTRY-1785 Fix TestKrbConnectionTimeout test

Posted by Vamsee Yarlagadda <va...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59576/#review176101
-----------------------------------------------------------


Ship it!




Ship It!

- Vamsee Yarlagadda


On May 25, 2017, 3:54 p.m., kalyan kumar kalvagadda wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59576/
> -----------------------------------------------------------
> 
> (Updated May 25, 2017, 3:54 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Brian Towles, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
> 
> 
> Bugs: SENTRY-1785
>     https://issues.apache.org/jira/browse/SENTRY-1785
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> There are couple of issues here
> 1. When Assume.assumeTrue fails the test is skipped but some times it is not skipped and the test fails.
> 2. setup(0 method in TestKrbConnectionTimeout calls setup again, which is reclusive call and there will be stack overflow.
> 
> Patch submitted has three changes
> 1. Updated the requires system property "sentry.hive.test.ticket.timeout" to true.
> 2. rename the BeforeClass annotated method of TestKrbConnectionTimeout class to avoid recursive call.
> 3. Invoked setup method to beginning of BeforeClass annotated method of TestKrbConnectionTimeout.
> 
> 
> Diffs
> -----
> 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestKrbConnectionTimeout.java b62a83f 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java dd37e7e 
> 
> 
> Diff: https://reviews.apache.org/r/59576/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Re: Review Request 59576: SENTRY-1785 Fix TestKrbConnectionTimeout test

Posted by Na Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59576/#review176091
-----------------------------------------------------------


Ship it!




Ship It!

- Na Li


On May 25, 2017, 3:54 p.m., kalyan kumar kalvagadda wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59576/
> -----------------------------------------------------------
> 
> (Updated May 25, 2017, 3:54 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Brian Towles, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
> 
> 
> Bugs: SENTRY-1785
>     https://issues.apache.org/jira/browse/SENTRY-1785
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> There are couple of issues here
> 1. When Assume.assumeTrue fails the test is skipped but some times it is not skipped and the test fails.
> 2. setup(0 method in TestKrbConnectionTimeout calls setup again, which is reclusive call and there will be stack overflow.
> 
> Patch submitted has three changes
> 1. Updated the requires system property "sentry.hive.test.ticket.timeout" to true.
> 2. rename the BeforeClass annotated method of TestKrbConnectionTimeout class to avoid recursive call.
> 3. Invoked setup method to beginning of BeforeClass annotated method of TestKrbConnectionTimeout.
> 
> 
> Diffs
> -----
> 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestKrbConnectionTimeout.java b62a83f 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java dd37e7e 
> 
> 
> Diff: https://reviews.apache.org/r/59576/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Re: Review Request 59576: SENTRY-1785 Fix TestKrbConnectionTimeout test

Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59576/#review176099
-----------------------------------------------------------


Fix it, then Ship it!




Ship It!


sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java
Lines 204 (patched)
<https://reviews.apache.org/r/59576/#comment249430>

    Why do we need to set this property to true? Can we just remove the assert that checks for it?


- Alexander Kolbasov


On May 25, 2017, 10:05 p.m., kalyan kumar kalvagadda wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59576/
> -----------------------------------------------------------
> 
> (Updated May 25, 2017, 10:05 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Brian Towles, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
> 
> 
> Bugs: SENTRY-1785
>     https://issues.apache.org/jira/browse/SENTRY-1785
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> There are couple of issues here
> 1. When Assume.assumeTrue fails the test is skipped but some times it is not skipped and the test fails.
> 2. setup(0 method in TestKrbConnectionTimeout calls setup again, which is reclusive call and there will be stack overflow.
> 
> Patch submitted has three changes
> 1. Updated the requires system property "sentry.hive.test.ticket.timeout" to true.
> 2. rename the BeforeClass annotated method of TestKrbConnectionTimeout class to avoid recursive call.
> 3. Invoked setup method to beginning of BeforeClass annotated method of TestKrbConnectionTimeout.
> 
> 
> Diffs
> -----
> 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestKrbConnectionTimeout.java b62a83f 
> 
> 
> Diff: https://reviews.apache.org/r/59576/diff/3/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Re: Review Request 59576: SENTRY-1785 Fix TestKrbConnectionTimeout test

Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59576/#review176150
-----------------------------------------------------------


Ship it!




Ship It!

- Alexander Kolbasov


On May 25, 2017, 10:05 p.m., kalyan kumar kalvagadda wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59576/
> -----------------------------------------------------------
> 
> (Updated May 25, 2017, 10:05 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Brian Towles, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
> 
> 
> Bugs: SENTRY-1785
>     https://issues.apache.org/jira/browse/SENTRY-1785
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> There are couple of issues here
> 1. When Assume.assumeTrue fails the test is skipped but some times it is not skipped and the test fails.
> 2. setup(0 method in TestKrbConnectionTimeout calls setup again, which is reclusive call and there will be stack overflow.
> 
> Patch submitted has three changes
> 1. Updated the requires system property "sentry.hive.test.ticket.timeout" to true.
> 2. rename the BeforeClass annotated method of TestKrbConnectionTimeout class to avoid recursive call.
> 3. Invoked setup method to beginning of BeforeClass annotated method of TestKrbConnectionTimeout.
> 
> 
> Diffs
> -----
> 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestKrbConnectionTimeout.java b62a83f 
> 
> 
> Diff: https://reviews.apache.org/r/59576/diff/3/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Re: Review Request 59576: SENTRY-1785 Fix TestKrbConnectionTimeout test

Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59576/
-----------------------------------------------------------

(Updated May 25, 2017, 10:05 p.m.)


Review request for sentry, Alexander Kolbasov, Brian Towles, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.


Changes
-------

skipped checking and setting the property property "sentry.hive.test.ticket.timeout".


Bugs: SENTRY-1785
    https://issues.apache.org/jira/browse/SENTRY-1785


Repository: sentry


Description
-------

There are couple of issues here
1. When Assume.assumeTrue fails the test is skipped but some times it is not skipped and the test fails.
2. setup(0 method in TestKrbConnectionTimeout calls setup again, which is reclusive call and there will be stack overflow.

Patch submitted has three changes
1. Updated the requires system property "sentry.hive.test.ticket.timeout" to true.
2. rename the BeforeClass annotated method of TestKrbConnectionTimeout class to avoid recursive call.
3. Invoked setup method to beginning of BeforeClass annotated method of TestKrbConnectionTimeout.


Diffs (updated)
-----

  sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestKrbConnectionTimeout.java b62a83f 


Diff: https://reviews.apache.org/r/59576/diff/2/

Changes: https://reviews.apache.org/r/59576/diff/1-2/


Testing
-------


Thanks,

kalyan kumar kalvagadda