Github user monsieurp commented on the issue:
https://github.com/apache/zeppelin/pull/3239
Yes it is! :)
---
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 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,
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 user monsieurp commented on the issue:
https://github.com/apache/zeppelin/pull/3239
@zhongneu: OK, done.
---
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
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 user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/3239
ping @monsieurp
---
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.
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 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 user monsieurp commented on the issue:
https://github.com/apache/zeppelin/pull/3239
Is there anything else I can improve?
---
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 user monsieurp commented on the issue:
https://github.com/apache/zeppelin/pull/3239
argh! I give up... :(
---
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 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 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
---
17 matches
Mail list logo
