You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by khalidhuseynov <gi...@git.apache.org> on 2016/08/06 07:50:28 UTC
[GitHub] zeppelin pull request #1293: [ZEPPELIN- 1298] Log instead of throwing trace ...
GitHub user khalidhuseynov opened a pull request:
https://github.com/apache/zeppelin/pull/1293
[ZEPPELIN- 1298] Log instead of throwing trace for ping messages
### What is this PR for?
When non authenticated user in non-anonymous mode tries to send any websocket api message it results in throwing a traceback. However on PING message we don't need to show the whole traceback and we can just log in order not to pollute the logs file.
### What type of PR is it?
Hot Fix
### Todos
* [ ] - log and return on PING
### What is the Jira issue?
[Zeppelin-1298](https://issues.apache.org/jira/browse/ZEPPELIN-1298)
### How should this be tested?
Follow the steps in issue and shouldn't get repeating traceback.
### Screenshots (if appropriate)
### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? no
in anonymous not allowed mode normally any nonauthenticated websocket message should throw an error, but ping doesn't do any job and no need to throw on it probably, just return and log
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/khalidhuseynov/incubator-zeppelin hotfix/zeppelin-1298
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/1293.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 #1293
----
commit 4df77242e68f1a1830374763e6feb986100fba06
Author: Khalid Huseynov <kh...@gmail.com>
Date: 2016-08-06T07:44:20Z
don't throw exception on ping
in anonymous not allowed mode normally any nonauthenticated websocket message should throw an error, but ping doesn't do any job and no need to throw on it probably, just return and 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] zeppelin issue #1293: [ZEPPELIN- 1298] Log instead of throwing trace for pin...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/1293
Merge into master and branch-0.6 if there're no further discussions.
---
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] zeppelin issue #1293: [ZEPPELIN- 1298] Log instead of throwing trace for pin...
Posted by khalidhuseynov <gi...@git.apache.org>.
Github user khalidhuseynov commented on the issue:
https://github.com/apache/zeppelin/pull/1293
@Leemoonsoo thanks for review. i addressed your comments by changing all the invalid cases to log in debug mode instead of throwing exception. let me know if there's something else left.
---
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] zeppelin issue #1293: [ZEPPELIN- 1298] Log instead of throwing trace for pin...
Posted by minahlee <gi...@git.apache.org>.
Github user minahlee commented on the issue:
https://github.com/apache/zeppelin/pull/1293
@khalidhuseynov Could you resolve conflicts?
---
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] zeppelin issue #1293: [ZEPPELIN- 1298] Log instead of throwing trace for pin...
Posted by khalidhuseynov <gi...@git.apache.org>.
Github user khalidhuseynov commented on the issue:
https://github.com/apache/zeppelin/pull/1293
@minahlee done
---
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] zeppelin pull request #1293: [ZEPPELIN- 1298] Log instead of throwing trace ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/zeppelin/pull/1293
---
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] zeppelin pull request #1293: [ZEPPELIN- 1298] Log instead of throwing trace ...
Posted by khalidhuseynov <gi...@git.apache.org>.
GitHub user khalidhuseynov reopened a pull request:
https://github.com/apache/zeppelin/pull/1293
[ZEPPELIN- 1298] Log instead of throwing trace for ping messages
### What is this PR for?
When non authenticated user in non-anonymous mode tries to send any websocket api message it results in throwing a traceback. However on PING message we don't need to show the whole traceback and we can just log in order not to pollute the logs file.
### What type of PR is it?
Hot Fix
### Todos
* [x] - log and return on PING
### What is the Jira issue?
[Zeppelin-1298](https://issues.apache.org/jira/browse/ZEPPELIN-1298)
### How should this be tested?
Follow the steps in issue and shouldn't get repeating traceback.
### 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/khalidhuseynov/incubator-zeppelin hotfix/zeppelin-1298
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/1293.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 #1293
----
commit 4df77242e68f1a1830374763e6feb986100fba06
Author: Khalid Huseynov <kh...@gmail.com>
Date: 2016-08-06T07:44:20Z
don't throw exception on ping
in anonymous not allowed mode normally any nonauthenticated websocket message should throw an error, but ping doesn't do any job and no need to throw on it probably, just return and 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] zeppelin issue #1293: [ZEPPELIN- 1298] Log instead of throwing trace for pin...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/1293
Thanks @khalidhuseynov for the fix. I tried this patch and it does not print stacktrace on PING message anymore when user not logged in.
However, it still prints stacktrace when user browse zeppelin gui. And i think it's also the case we don't need to print stacktrace.
Could you handle this case, too?
And IMO, i think it's okay to adjust log message level to DEBUG for both case because user browse Zeppelin gui without valid ticket is fairly normal behavior, when they're not logged in.
---
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] zeppelin issue #1293: [ZEPPELIN- 1298] Log instead of throwing trace for pin...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/1293
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] zeppelin pull request #1293: [ZEPPELIN- 1298] Log instead of throwing trace ...
Posted by khalidhuseynov <gi...@git.apache.org>.
Github user khalidhuseynov closed the pull request at:
https://github.com/apache/zeppelin/pull/1293
---
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] zeppelin issue #1293: [ZEPPELIN- 1298] Log instead of throwing trace for pin...
Posted by bzz <gi...@git.apache.org>.
Github user bzz commented on the issue:
https://github.com/apache/zeppelin/pull/1293
`LOG.debug` sounds like a great idea to me to avoid filling up the logs in default configuration.
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.
---