You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@livy.apache.org by "dacort (via GitHub)" <gi...@apache.org> on 2023/03/13 22:26:37 UTC
[GitHub] [incubator-livy] dacort opened a new pull request, #392: initial attempt at reload4j and excluding transitive dependencies
dacort opened a new pull request, #392:
URL: https://github.com/apache/incubator-livy/pull/392
## What changes were proposed in this pull request?
- Replace log4j with reload4j
- Add exclusions for transitive log4j dependencies including `spark-core`, `hadoop-common` (and others), `zookeeper`, and `apacheds-server`.
- https://issues.apache.org/jira/browse/LIVY-878
## How was this patch tested?
- unit/integration tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] dacort commented on pull request #392: [LIVY-878] Replace log4j with reload4j
Posted by "dacort (via GitHub)" <gi...@apache.org>.
dacort commented on PR #392:
URL: https://github.com/apache/incubator-livy/pull/392#issuecomment-1505601797
Might have figured this out! Swapping `slf4j-api` with `slf4j-reload4j` seems to get me past `TestSparkClient`. Running the full suite now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] ayushtkn commented on pull request #392: [LIVY-878] Replace log4j with reload4j
Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.
ayushtkn commented on PR #392:
URL: https://github.com/apache/incubator-livy/pull/392#issuecomment-1483189148
If it isn't getting used, its better if we could exclude the log4j dependency completely even from test scope
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] dacort commented on pull request #392: [LIVY-878] Replace log4j with reload4j
Posted by "dacort (via GitHub)" <gi...@apache.org>.
dacort commented on PR #392:
URL: https://github.com/apache/incubator-livy/pull/392#issuecomment-1500961788
Will trigger CI with a push today.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] dacort commented on pull request #392: [LIVY-878] Replace log4j with reload4j
Posted by "dacort (via GitHub)" <gi...@apache.org>.
dacort commented on PR #392:
URL: https://github.com/apache/incubator-livy/pull/392#issuecomment-1503829916
Fixed the `java.lang.NoClassDefFoundError` error with additional exclusions.
Unit tests are failing, however I think it's due to a flaky test based on behavior observed in CI where unit tests pass on the PR but fail once merged into master and vice-cersa.
There's an old [mailing list thread](https://www.mail-archive.com/dev@livy.incubator.apache.org/msg00794.html) that also discusses the issue, but with no resolution.
```
The second error, TestSparkClient.testJobSubmission, is random. I see the
failure ~25% of the times. As it turned out, the Echo job was returning,
sometimes, even before we added the listener on the job handle. So the
listener.onJobStarted method is not invoked. This can be fixed in multiple
ways but I updated the Echo job, which used to simply return its attribute
value, to do what ScalaEcho job is doing. So the Echo job now returns
"jc.sc().parallelize(list,
1).collect().get(0);". Even this could still be fast enough to cause the
same error. However I couldn't reproduce it again. Also the updated code is
taking ~1 sec or more on spark's side. So hopefully this should be ok.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] dacort commented on pull request #392: [LIVY-878] Replace log4j with reload4j
Posted by "dacort (via GitHub)" <gi...@apache.org>.
dacort commented on PR #392:
URL: https://github.com/apache/incubator-livy/pull/392#issuecomment-1505278667
I've been poking at the failure for this change more and turns out it's something different. When I add exclusions for `slf4j-log4j12` to `spark-core`, Netty fails to find slf4j logger, instead tries to use log4j and, I think, ends up failing. If I don't add that exclusion, the test runs fine.
I'm still trying to figure out what exactly is going on there but not having much luck.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] ksumit commented on pull request #392: [LIVY-878] Replace log4j with reload4j
Posted by "ksumit (via GitHub)" <gi...@apache.org>.
ksumit commented on PR #392:
URL: https://github.com/apache/incubator-livy/pull/392#issuecomment-1523745343
Given that this has taken us quiet some time, i do want to checkpoint this work and iterate over if there are issues. Approving for check-in.
+1, LGTM
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] gyogal commented on pull request #392: [LIVY-878] Replace log4j with reload4j
Posted by "gyogal (via GitHub)" <gi...@apache.org>.
gyogal commented on PR #392:
URL: https://github.com/apache/incubator-livy/pull/392#issuecomment-1505025583
Interesting, this test does seem to be flaky. The recent post-commit tests have succeeded, but that may be just pure luck. Could you please retrigger the tests on this one? If this test proves to be flaky, we should file a separate JIRA for it. Also, thanks for finding that old email thread about this test!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] dacort merged pull request #392: [LIVY-878] Replace log4j with reload4j
Posted by "dacort (via GitHub)" <gi...@apache.org>.
dacort merged PR #392:
URL: https://github.com/apache/incubator-livy/pull/392
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] lmccay commented on pull request #392: [LIVY-878] Replace log4j with reload4j
Posted by "lmccay (via GitHub)" <gi...@apache.org>.
lmccay commented on PR #392:
URL: https://github.com/apache/incubator-livy/pull/392#issuecomment-1520865529
@dacort - curious where you are with the separate branch and plan here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] dacort commented on pull request #392: [LIVY-878] Replace log4j with reload4j
Posted by "dacort (via GitHub)" <gi...@apache.org>.
dacort commented on PR #392:
URL: https://github.com/apache/incubator-livy/pull/392#issuecomment-1522662904
@lmccay @ksumit OK! I've merged in changes from my working branch. Tests are green! (After some manual retries...)
Could y'all take one more glance at this and the I think it's good to merge in. Apologies this has taken so long, appreciate your patience and follow ups. 🙏
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] dacort commented on pull request #392: [LIVY-878] Replace log4j with reload4j
Posted by "dacort (via GitHub)" <gi...@apache.org>.
dacort commented on PR #392:
URL: https://github.com/apache/incubator-livy/pull/392#issuecomment-1523744674
> slf4j-reload4j as provided dependency
I'll check, but it is defined as a non-provided dependency in the main `pom.xml`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] dacort commented on pull request #392: [LIVY-878] Replace log4j with reload4j
Posted by "dacort (via GitHub)" <gi...@apache.org>.
dacort commented on PR #392:
URL: https://github.com/apache/incubator-livy/pull/392#issuecomment-1500992072
Bummer, CI failed. Will take a look either later today or Monday.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] dacort commented on pull request #392: [LIVY-878] Replace log4j with reload4j
Posted by "dacort (via GitHub)" <gi...@apache.org>.
dacort commented on PR #392:
URL: https://github.com/apache/incubator-livy/pull/392#issuecomment-1501165029
Integration tests are working, but unit tests still failing with, what I think is an error related to slf4j/log4j loading.
```
[INFO] Running org.apache.livy.thriftserver.session.ColumnBufferTest
[ERROR] Tests run: 5, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 2.572 s <<< FAILURE! - in org.apache.livy.thriftserver.session.ColumnBufferTest
[ERROR] testColumnBuffer(org.apache.livy.thriftserver.session.ColumnBufferTest) Time elapsed: 2.147 s <<< ERROR!
java.lang.NoClassDefFoundError: org/apache/log4j/Level
at org.apache.livy.thriftserver.session.ColumnBufferTest.testColumnBuffer(ColumnBufferTest.java:53)
Caused by: java.lang.ClassNotFoundException: org.apache.log4j.Level
at org.apache.livy.thriftserver.session.ColumnBufferTest.testColumnBuffer(ColumnBufferTest.java:53)
[INFO] Running org.apache.livy.thriftserver.session.ThriftSessionTest
[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.024 s <<< FAILURE! - in org.apache.livy.thriftserver.session.ThriftSessionTest
[ERROR] org.apache.livy.thriftserver.session.ThriftSessionTest Time elapsed: 0.024 s <<< ERROR!
java.lang.ExceptionInInitializerError
at org.apache.livy.thriftserver.session.ThriftSessionTest.setUp(ThriftSessionTest.java:60)
Caused by: java.lang.IllegalStateException: org.slf4j.LoggerFactory in failed state. Original exception was thrown EARLIER. See also http://www.slf4j.org/codes.html#unsuccessfulInit
at org.apache.livy.thriftserver.session.ThriftSessionTest.setUp(ThriftSessionTest.java:60)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] dacort commented on pull request #392: [LIVY-878] Replace log4j with reload4j
Posted by "dacort (via GitHub)" <gi...@apache.org>.
dacort commented on PR #392:
URL: https://github.com/apache/incubator-livy/pull/392#issuecomment-1500721635
AhI thought I addressed the `apacheds-server-integ` - will double-check that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] codecov-commenter commented on pull request #392: [LIVY-878] Replace log4j with reload4j
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #392:
URL: https://github.com/apache/incubator-livy/pull/392#issuecomment-1500991826
## [Codecov](https://codecov.io/gh/apache/incubator-livy/pull/392?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#392](https://codecov.io/gh/apache/incubator-livy/pull/392?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7ff3ec1) into [master](https://codecov.io/gh/apache/incubator-livy/commit/8d07eee831221093f55203190d38d793234c5db7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8d07eee) will **decrease** coverage by `40.64%`.
> The diff coverage is `75.60%`.
> :exclamation: Current head 7ff3ec1 differs from pull request most recent head a59259c. Consider uploading reports for the commit a59259c to get more accurate results
```diff
@@ Coverage Diff @@
## master #392 +/- ##
=============================================
- Coverage 68.26% 27.63% -40.64%
+ Complexity 844 362 -482
=============================================
Files 103 103
Lines 5965 5989 +24
Branches 907 911 +4
=============================================
- Hits 4072 1655 -2417
- Misses 1333 3988 +2655
+ Partials 560 346 -214
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-livy/pull/392?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...cala/org/apache/livy/sessions/SessionManager.scala](https://codecov.io/gh/apache/incubator-livy/pull/392?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9zZXNzaW9ucy9TZXNzaW9uTWFuYWdlci5zY2FsYQ==) | `63.72% <50.00%> (-18.10%)` | :arrow_down: |
| [...java/org/apache/livy/client/common/ClientConf.java](https://codecov.io/gh/apache/incubator-livy/pull/392?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50LWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvbGl2eS9jbGllbnQvY29tbW9uL0NsaWVudENvbmYuamF2YQ==) | `69.36% <62.50%> (-29.70%)` | :arrow_down: |
| [.../server/interactive/CreateInteractiveRequest.scala](https://codecov.io/gh/apache/incubator-livy/pull/392?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9zZXJ2ZXIvaW50ZXJhY3RpdmUvQ3JlYXRlSW50ZXJhY3RpdmVSZXF1ZXN0LnNjYWxh) | `62.50% <66.66%> (-17.50%)` | :arrow_down: |
| [...server/interactive/InteractiveSessionServlet.scala](https://codecov.io/gh/apache/incubator-livy/pull/392?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9zZXJ2ZXIvaW50ZXJhY3RpdmUvSW50ZXJhY3RpdmVTZXNzaW9uU2VydmxldC5zY2FsYQ==) | `58.01% <71.42%> (-9.71%)` | :arrow_down: |
| [.../scala/org/apache/livy/server/SessionServlet.scala](https://codecov.io/gh/apache/incubator-livy/pull/392?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9zZXJ2ZXIvU2Vzc2lvblNlcnZsZXQuc2NhbGE=) | `52.57% <85.71%> (-19.01%)` | :arrow_down: |
| [...va/org/apache/livy/client/common/HttpMessages.java](https://codecov.io/gh/apache/incubator-livy/pull/392?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50LWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvbGl2eS9jbGllbnQvY29tbW9uL0h0dHBNZXNzYWdlcy5qYXZh) | `81.39% <100.00%> (-13.85%)` | :arrow_down: |
| [...e/livy/server/interactive/InteractiveSession.scala](https://codecov.io/gh/apache/incubator-livy/pull/392?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9zZXJ2ZXIvaW50ZXJhY3RpdmUvSW50ZXJhY3RpdmVTZXNzaW9uLnNjYWxh) | `66.28% <100.00%> (-3.49%)` | :arrow_down: |
| [.../main/scala/org/apache/livy/sessions/Session.scala](https://codecov.io/gh/apache/incubator-livy/pull/392?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9zZXNzaW9ucy9TZXNzaW9uLnNjYWxh) | `63.47% <100.00%> (-9.98%)` | :arrow_down: |
... and [74 files with indirect coverage changes](https://codecov.io/gh/apache/incubator-livy/pull/392/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] lmccay commented on pull request #392: [LIVY-878] Replace log4j with reload4j
Posted by "lmccay (via GitHub)" <gi...@apache.org>.
lmccay commented on PR #392:
URL: https://github.com/apache/incubator-livy/pull/392#issuecomment-1513881909
@dacort - looks like the same failures?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] dacort commented on pull request #392: [LIVY-878] Replace log4j with reload4j
Posted by "dacort (via GitHub)" <gi...@apache.org>.
dacort commented on PR #392:
URL: https://github.com/apache/incubator-livy/pull/392#issuecomment-1513888628
@lmccay Yes but I've got another working branch over here https://github.com/apache/incubator-livy/tree/fix/log4j-reload4j-part2 that appears to be working. Just need to verify everything looks good.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org