You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by zhongneu <gi...@git.apache.org> on 2016/02/28 05:49:25 UTC
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-679] only print the err...
GitHub user zhongneu opened a pull request:
https://github.com/apache/incubator-zeppelin/pull/754
[ZEPPELIN-679] only print the error message for SQL exceptions
### What is this PR for?
Most of the SQL exceptions are syntax errors. Printing the full stack trace is not necessary. We should only print the error message to avoid distracting the users
### What type of PR is it?
Improvement
### Todos
### What is the Jira issue?
[ZEPPELIN-679](https://issues.apache.org/jira/browse/ZEPPELIN-679)
### How should this be tested?
### Screenshots (if appropriate)
### Questions:
* Does the licenses files need update?
NO
* Is there breaking changes for older versions?
NO
* Does this needs documentation?
NO
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zhongneu/incubator-zeppelin less-sql-err-msg
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-zeppelin/pull/754.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #754
----
commit 39dd274d76a7f3cf1ee736215a08e17ca2e27593
Author: Zhong Wang <wa...@gmail.com>
Date: 2016-02-27T07:27:02Z
only print the error message for SQL exceptions
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-679] only print the err...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/754#issuecomment-192670762
LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-679] only print the err...
Posted by echarles <gi...@git.apache.org>.
Github user echarles commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/754#issuecomment-189898912
I often use the stacktraces to debug what's going wrong. With this, I expect I will miss information. Can we make this configurable (default being "not verbose")?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-679] only print the err...
Posted by zhongneu <gi...@git.apache.org>.
Github user zhongneu commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/754#issuecomment-190604656
@echarles Thanks, Eric! It is good to know we have such cases. We should definitely leave it as configurable if Spark doesn't provide informative error message. I am not sure, but probably in older versions of Spark, there may be more such cases.
Just curious, what kind of queries can result in such exception?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-679] only print the err...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/754#issuecomment-192706297
LGTM. @echarles what do you think?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-679] only print the err...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-zeppelin/pull/754
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-679] only print the err...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/754#issuecomment-189787467
thx - let's add logger to capture the full stack before returning with InterpreterResult?
```
logger.error("Invocation target exception", e);
```
?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-679] only print the err...
Posted by echarles <gi...@git.apache.org>.
Github user echarles commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/754#issuecomment-193657371
@AhyoungRyu I have opened ZEPPELIN-725 for the interpreter update issue.
@felixcheung @Leemoonsoo This PR #754 is good to merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-679] only print the err...
Posted by zhongneu <gi...@git.apache.org>.
Github user zhongneu commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/754#issuecomment-189788087
@felixcheung great idea. updated & tested. full stack trace will be logged in the interpreter log
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-679] only print the err...
Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/754#issuecomment-193660503
@echarles Thanks for creating the issue!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-679] only print the err...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/754#issuecomment-189801164
LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-679] only print the err...
Posted by zhongneu <gi...@git.apache.org>.
Github user zhongneu commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/754#issuecomment-189806808
fixed the test
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-679] only print the err...
Posted by zhongneu <gi...@git.apache.org>.
Github user zhongneu commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/754#issuecomment-189950117
@echarles I thought about made this configurable, but I thought the stacktraces might not be helpful for SQL queries, because typically it is difficult for the users to dig into the internal Spark implementation. Could you give me examples of using stacktraces to debug SQL queries?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-679] only print the err...
Posted by zhongneu <gi...@git.apache.org>.
Github user zhongneu commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/754#issuecomment-192100210
Added a configuration to show full stacktrace
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-679] only print the err...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/754#issuecomment-189802314
btw, it looks like this might be a legitimate test failure, could you take a look?
https://s3.amazonaws.com/archive.travis-ci.org/jobs/112358128/log.txt
```
Failed tests:
SparkSqlInterpreterTest.test:110 Exception not catched
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-679] only print the err...
Posted by joelvanveluwen <gi...@git.apache.org>.
Github user joelvanveluwen commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/754#issuecomment-190607278
+1 for making it configurable. I find the stack trace invaluable!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-679] only print the err...
Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/754#issuecomment-193308252
@echarles I faced the same issue. I think it's a bug..
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-679] only print the err...
Posted by echarles <gi...@git.apache.org>.
Github user echarles commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/754#issuecomment-190609552
So we all agree to make it configurable on the interpreter settings leve
`zeppelin.spark.sql.stacktrace = false` by default ?
What about when you catch and retrhow the expection to append a message like "Configure zeppelin.spark.sql.stacktrace = true to see the stacktrace, or use sqlc.sql(query)"
We would have best of both worlds.
@ zhongneu don't have the query at hand, anyway, the stacktrace helped me to identify an bug unrelated to sql but related to storage access.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-679] only print the err...
Posted by zhongneu <gi...@git.apache.org>.
Github user zhongneu commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/754#issuecomment-190611618
I agree to make it configurable, but show the shortened message by default
@echarles I think that's a good suggestion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-679] only print the err...
Posted by echarles <gi...@git.apache.org>.
Github user echarles commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/754#issuecomment-193176072
Tested with zeppelin.spark.sql.stacktrace being false and true, and it works fine.
Good to merge.
PS: With HEAD source, I have issues settings any property via the UI (When I restart, the changed values are not taken into account) - Had to set them changing the interpreter.json. Is this only me on my env?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-679] only print the err...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/754#issuecomment-194027542
Thanks i'm merging it into master.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-679] only print the err...
Posted by nemccarthy <gi...@git.apache.org>.
Github user nemccarthy commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/754#issuecomment-190606798
this seems a little misguided. hiding errors is great until you need to stack trace.
id vote to have it configurable where the default is always full stack traces are visible.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-679] only print the err...
Posted by echarles <gi...@git.apache.org>.
Github user echarles commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/754#issuecomment-190602637
@zhongneu I will rather give you a message a user could face running the SQL: `java.lang.IllegalArgumentException: requirement failed` What will the guy responsible to troubleshoot this ticket will do without the stacktrace?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-679] only print the err...
Posted by zhongneu <gi...@git.apache.org>.
Github user zhongneu commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/754#issuecomment-189785825
@felixcheung definitely. updated based on your comment
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---