[GitHub] drill issue #1024: DRILL-3640: Support JDBC Statement.setQueryTimeout(int)

2018-01-24 Thread parthchandra 
Github user parthchandra commented on the issue:

https://github.com/apache/drill/pull/1024
  
+1. Look like there are no more review comments to be addressed.


---

[GitHub] drill issue #1024: DRILL-3640: Support JDBC Statement.setQueryTimeout(int)

2018-01-22 Thread kkhatua 
Github user kkhatua commented on the issue:

https://github.com/apache/drill/pull/1024
  
Did a rebase on the latest master to resolve the merge conflict from 
DRILL-3993 
[commit](https://github.com/apache/drill/commit/9fabb612f16f6f541b3bde68ad7d734cad26df33#diff-f5de5223afdaaec6d009c4e06015e34d)
 that upgraded to Calcite 1.13.


---

[GitHub] drill issue #1024: DRILL-3640: Support JDBC Statement.setQueryTimeout(int)

2018-01-04 Thread kkhatua 
Github user kkhatua commented on the issue:

https://github.com/apache/drill/pull/1024
  
Ready for a final review. 
All unit tests, with the exception of 
`PreparedStatementTest.testServerTriggeredQueryTimeout` . The test is being 
ignored because the timed pause injection is not being honoured for a 
PreparedStatement, though it is honoured for a regular Statement. In actual 
dev/functional testing, however, the timeout works, which makes me believe 
there is a limitation with the test framework injecting pauses for Prepared 
Statement.


---

[GitHub] drill issue #1024: DRILL-3640: Support JDBC Statement.setQueryTimeout(int)

2017-11-29 Thread kkhatua 
Github user kkhatua commented on the issue:

https://github.com/apache/drill/pull/1024
  
@laurentgo 
I've added server-triggered timeout tests and made other changes as well, 
but they require support for 
[DRILL-5973](https://issues.apache.org/jira/browse/DRILL-5973) . I tested this 
commit (#1024 ) as a cherry pick on top of that PR's commit (#1055) and I was 
able to simulate the server-induced timeout.
Will need a +1 for that PR before I can enable the tests here.
For now, I've marked these tests as `@ignore` to ensure that the remaining 
tests pass and the feature works as intended. 

Can you review them both (this and #1055 )?


---

[GitHub] drill issue #1024: DRILL-3640: Support JDBC Statement.setQueryTimeout(int)

2017-11-07 Thread kkhatua 
Github user kkhatua commented on the issue:

https://github.com/apache/drill/pull/1024
  
@laurentgo Done the changes... ready for review. 


---

[GitHub] drill issue #1024: DRILL-3640: Support JDBC Statement.setQueryTimeout(int)

2017-11-03 Thread kkhatua 
Github user kkhatua commented on the issue:

https://github.com/apache/drill/pull/1024
  
@parthchandra / @laurentgo 
I've implemented changes based on the conversation in PR 
https://github.com/apache/drill/pull/858 and waiting for your review of this 
commit.


---