[GitHub] zeppelin issue #3239: Zeppelin 3879: create "maxRows" and "rowsFetchSize" va...
Github user monsieurp commented on the issue: https://github.com/apache/zeppelin/pull/3239 Yes it is! :) ---
[GitHub] zeppelin issue #3239: Zeppelin 3879: create "maxRows" and "rowsFetchSize" va...
Github user zjffdu commented on the issue: https://github.com/apache/zeppelin/pull/3239 @monsieurp Is `Patrice Clement` your jira account ? I'd like to assign this ticket to you. ---
[GitHub] zeppelin issue #3239: Zeppelin 3879: create "maxRows" and "rowsFetchSize" va...
Github user zjffdu commented on the issue: https://github.com/apache/zeppelin/pull/3239 No problem, Thanks @monsieurp , will merge this PR soon. BTW, could you help create a ticket for removing `common.max_count` and welcome to contribute again ð ---
[GitHub] zeppelin issue #3239: Zeppelin 3879: create "maxRows" and "rowsFetchSize" va...
Github user monsieurp commented on the issue: https://github.com/apache/zeppelin/pull/3239 @zffdu: Sorry for not responding earlier. IMHO, deleting `common.max_count` should be dealt with in a separate PR / bug report since it is completely unrelated to the feature here. Until then, can we merge this PR? Thanks! ---
[GitHub] zeppelin issue #3239: Zeppelin 3879: create "maxRows" and "rowsFetchSize" va...
Github user zjffdu commented on the issue: https://github.com/apache/zeppelin/pull/3239 Thanks @monsieurp Could you also delete `common.max_count` from `interpreter-setting.json` and test code ? ---
[GitHub] zeppelin issue #3239: Zeppelin 3879: create "maxRows" and "rowsFetchSize" va...
Github user monsieurp commented on the issue: https://github.com/apache/zeppelin/pull/3239 @zhongneu: OK, done. ---
[GitHub] zeppelin issue #3239: Zeppelin 3879: create "maxRows" and "rowsFetchSize" va...
Github user zjffdu commented on the issue: https://github.com/apache/zeppelin/pull/3239 It is weird that I see another property `common.max_count` but it is never used in `JdbcInterpreter`. I think we just need to keep one and remove others. Personally I prefer `zeppelin.jdbc.maxRows` ---
[GitHub] zeppelin issue #3239: Zeppelin 3879: create "maxRows" and "rowsFetchSize" va...
Github user monsieurp commented on the issue: https://github.com/apache/zeppelin/pull/3239 @zjffdu: Hi! Fair enough. What do you want me to do then? Should I remove the `rowsFetchSize` property? ---
[GitHub] zeppelin issue #3239: Zeppelin 3879: create "maxRows" and "rowsFetchSize" va...
Github user zjffdu commented on the issue: https://github.com/apache/zeppelin/pull/3239 ping @monsieurp ---
[GitHub] zeppelin issue #3239: Zeppelin 3879: create "maxRows" and "rowsFetchSize" va...
Github user zjffdu commented on the issue: https://github.com/apache/zeppelin/pull/3239 @monsieurp I though about it again, and feel that providing 2 properties is very confusing to users. I think it is better to give one property (zeppelin.jdbc.maxRows) what is user care about. `zeppelin.jdbc.rowsFetchSize` is internal property that zeppelin should configure to users. Here's one post explain the difference between `setFetchSize` and `setMaxRows` https://stackoverflow.com/questions/32399546/what-is-the-difference-between-statement-setmaxrows-vs-statement-setfetchsize-in?lq=1 ---
[GitHub] zeppelin issue #3239: Zeppelin 3879: create "maxRows" and "rowsFetchSize" va...
Github user monsieurp commented on the issue: https://github.com/apache/zeppelin/pull/3239 @zjffdu: Sorry for taking so long to respond. It's been a busy week. I've rebased my PR as requested. ---
[GitHub] zeppelin issue #3239: Zeppelin 3879: create "maxRows" and "rowsFetchSize" va...
Github user zjffdu commented on the issue: https://github.com/apache/zeppelin/pull/3239 @monsieurp Could you do a rebase and retrigger the travis build ? ---
[GitHub] zeppelin issue #3239: Zeppelin 3879: create "maxRows" and "rowsFetchSize" va...
Github user monsieurp commented on the issue: https://github.com/apache/zeppelin/pull/3239 Is there anything else I can improve? ---
[GitHub] zeppelin issue #3239: Zeppelin 3879: create "maxRows" and "rowsFetchSize" va...
Github user zjffdu commented on the issue: https://github.com/apache/zeppelin/pull/3239 @monsieurp This might be flaky test, let me take a look at it. ---
[GitHub] zeppelin issue #3239: Zeppelin 3879: create "maxRows" and "rowsFetchSize" va...
Github user monsieurp commented on the issue: https://github.com/apache/zeppelin/pull/3239 argh! I give up... :( ---
[GitHub] zeppelin issue #3239: Zeppelin 3879: create "maxRows" and "rowsFetchSize" va...
Github user monsieurp commented on the issue: https://github.com/apache/zeppelin/pull/3239 I don't know why Jenkins failed. Let's try again. ---
[GitHub] zeppelin issue #3239: Zeppelin 3879: create "maxRows" and "rowsFetchSize" va...
Github user monsieurp commented on the issue: https://github.com/apache/zeppelin/pull/3239 Thanks @zjffdu and @aka-demik for your comments. I've updated the PR accordingly. ---
[GitHub] zeppelin issue #3239: Zeppelin 3879: create "maxRows" and "rowsFetchSize" va...
Github user zjffdu commented on the issue: https://github.com/apache/zeppelin/pull/3239 Thanks @monsieurp for the contribution, I left a few comments, otherwise LGTM ---
