You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by revans2 <gi...@git.apache.org> on 2015/04/28 18:32:34 UTC
[GitHub] storm pull request: STORM-803: Better CI logs
GitHub user revans2 opened a pull request:
https://github.com/apache/storm/pull/539
STORM-803: Better CI logs
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/revans2/incubator-storm STORM-803
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/539.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 #539
----
commit fea23093369b1881363b44ab7b1875b475f25e14
Author: Robert (Bobby) Evans <ev...@yahoo-inc.com>
Date: 2015-04-27T19:28:15Z
STORM-803: Better CI logs
----
---
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] storm pull request: STORM-803: Better CI logs
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/539#issuecomment-98848483
@revans2 I didn't think about it yet, but if it is possible it'll be great!
---
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] storm pull request: STORM-803: Better CI logs
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/539#issuecomment-97223969
It doesn't mean that I don't agree only WARN log is enough.
I strongly agree we need INFO log to trace what happened.
For example, multilang tests with Travis CI may run fine but just makes some noise at cleanup.
With WARN logs we can't determine Topology runs properly cause we relied on 'topology.debug: true' and it prints out debug messages to INFO.
---
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] storm pull request: STORM-803: Better CI logs
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/539#issuecomment-113232324
Should be good now with all of the changes.
---
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] storm pull request: STORM-803: Better CI logs
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/539#issuecomment-97958909
Btw, restricting log level to WARN level is only applied to storm-core now, since I didn't add logback-test.xml on any other modules. Other modules don't print out much so it can be fully printed without issues.
---
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] storm pull request: STORM-803: Better CI logs
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/539#issuecomment-112917561
OK The updated test runner should be ready for review now.
---
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] storm pull request: STORM-803: Better CI logs
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/539#issuecomment-97212547
LGTM on adding all test reports to print out issues!
Btw, it seems to be regression.
I've thought about logging to file, but if we can't access log file or can't print part of logs which points to failed build, it isn't informative anymore.
As we can see, printing "system-out" and "system-err" doesn't work at least Kafka tests.
Could we upload log file to somewhere?
Travis CI has a plugin which uploads artifact to S3, so we can use it.
Just I don't know about "free storage" and it should be accessed via http for easy seeing.
---
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] storm pull request: STORM-803: Better CI logs
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/539#issuecomment-113206551
I like the CDATA and will look at switching over to that.
---
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] storm pull request: STORM-803: Better CI logs
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/539#issuecomment-113228802
I updated the code to use CDATA, and it is a lot better now. I also found some race conditions where log lines could leak into the XML file from background threads, which would cause the parsing of the file to fail. That is fixed now, and should be ready for review.
---
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] storm pull request: STORM-803: Better CI logs
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/539#issuecomment-97890019
@HeartSaVioR I agree that some things may be lost when we write the logs out to a file. That is why I added in the `cat` on failure when compiling, and the `tail -500` on a test failure. So if everything passed there is no issue, but if something failed we can get something out, even if it is not everything. If you want to adjust the amount output on failure I am happy to.
I have also been looking at the clojure test runner, but I am really not ready to update it yet. In most cases for me I have found this more useful at finding errors than the WARN has been, but that is just me.
---
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] storm pull request: STORM-803: Better CI logs
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/539#issuecomment-113229613
Oh I forgot the kafka change.
---
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] storm pull request: STORM-803: Better CI logs
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/539
---
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] storm pull request: STORM-803: Better CI logs
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/539#issuecomment-97956783
Yes, right. We need INFO log since Storm debug messages are printed out INFO.
Maybe your approach requires Maven to fail fast (--ff), which I didn't used it yet.
You can see build 20.1 has storm-kafka test error, but it only prints out storm-hive logs.
Btw, whatever if it works like a charm, we may want to run tests continuously, cause we can't retry build for now.
Actually collaborators - committers can retry build at Travis CI UI page, but it checks permission on github repo. Is it a rule that only asfgit has write access to github repo.?
---
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] storm pull request: STORM-803: Better CI logs
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/539#issuecomment-98843966
@HeartSaVioR I agree. I have shifted my focus to looking at how to improve the clojure maven plugin so we can capture the test results and display them properly. Once my pull request there goes in I'll try to update this one to take advantage of some of the changes. After that we should be able to get stdout/stderr only for tests that failed.
---
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] storm pull request: STORM-803: Better CI logs
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/539#issuecomment-97148798
The CI failures were both caused by kafka, and are not related to this change at all.
---
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] storm pull request: STORM-803: Better CI logs
Posted by knusbaum <gi...@git.apache.org>.
Github user knusbaum commented on the pull request:
https://github.com/apache/storm/pull/539#issuecomment-115376451
+1
---
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] storm pull request: STORM-803: Better CI logs
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/539#issuecomment-113253559
Great! +1.
---
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] storm pull request: STORM-803: Better CI logs
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/539#issuecomment-112945204
@revans2
Far more better! Really amazing job, I appreciate it.
Btw, I have some points to ask / report.
- I excluded storm-kafka tests from Travis CI build (STORM-864), and github diff shows that you revived storm-kafka.
I'd like to exclude it again, since tests in storm-kafka fails often, and it leads that we can't believe Travis CI result. (Travis CI notifies us your PR is broken but it's all about test failures of storm-kafka) Or do you have some ideas to fix?
- drpc test report seems broken. There's some text between testcase tag.
https://travis-ci.org/apache/storm/jobs/67249769
```
175150 [main] INFO b.s.testing - Shutting down in process zookeeper
175159 [main] INFO b.s.testing - Done shutting down in process zookeeper
175160 [main] INFO b.s.testing - Deleting temporary path /tmp/c4a2492e-6759-42c8-80bc-a48f42551e61
175161 [main] INFO b.s.testing - Deleting temporary path /tmp/b9c776d0-5e9f-41b4-9f4e-dfd97669ae05
175161 [main] INFO b.s.testing - Deleting temporary path /tmp/29bfa7dd-1a51-4561-8519-9a5355886ab6
175165 [main] INFO b.s.testing - Deleting temporary path /tmp/1884de6c-49ae-4ca3-9b1f-0b7975420b56
</system-out>
</testcase>
175166 [Thread-851] INFO b.s.util - Async loop interrupted!
<testcase name="test-drpc-flow" classname="backtype.storm.drpc-test">
<system-out>
175168 [main] INFO b.s.u.Utils - Using defaults.yaml from resources
175178 [main] INFO b.s.zookeeper - Starting inprocess zookeeper at port 2002 and dir /tmp/ebe0c08f-cf7b-47c6-9b24-123c16519c82
175181 [main] INFO b.s.u.Utils - Using defaults.yaml from resources
175190 [main] INFO b.s.d.nimbus - Starting Nimbus with conf {"topology.builtin.metrics.bucket.size.secs" 60, "nimbus.childopts" "-Xmx1024m", "ui.filter.params" nil,
```
- If we can apply CDATA to report XML it would be more fine. Escaped characters disturbs readability slightly.
---
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.
---