Skip to content

Conversation

@kirklund
Copy link
Contributor

@kirklund kirklund commented Jul 17, 2020

Upgrade ClassGraph dependency from 4.8.52 to 4.8.87.

See GEODE-8150. There were performance issues with 4.8.78 that caused us to rollback to 4.8.52. We'll need to do extra testing as well as review of ClassGraph release notes and bugs.

The intention is to review and fully test any new release of ClassGraph with bug fixes to determine if it's worth upgrading the dependency in Geode. If it introduces new problems or doesn't offer any improvements, we won't upgrade. If it's more stable and fixes bugs in ClassGraph that Geode is vulnerable to, then I will recommend upgrading.

ClassGraph release notes: https://github.com/classgraph/classgraph/releases
ClassGraph closed bugs: https://github.com/classgraph/classgraph/issues?q=is%3Aissue+is%3Aclosed

CAUTION: Please don't merge this PR for me!

@kirklund kirklund force-pushed the GEODE-8368-ClassGraph-4.8.87 branch 2 times, most recently from 67a9f88 to 08a864e Compare July 20, 2020 22:35
@onichols-pivotal
Copy link
Contributor

Out of curiosity what are we changing this? Was there a specific issue with 4.8.52 you are trying to solve? I would much rather go back to 4.0.6 and fix the log4j issue with an exclusion, than mess around with any of these newfangled versions.

@kirklund kirklund force-pushed the GEODE-8368-ClassGraph-4.8.87 branch from 08a864e to 9e97e37 Compare July 21, 2020 21:19
@kirklund
Copy link
Contributor Author

@onichols-pivotal ClassGraph has nothing to do with Log4j. I analyzed a test run in which a thread hung in ClassGraph#scan. ClassGraph has had many bugs identified and fixed between 4.8.52 and 4.8.87. I believe that a dependency like this should be kept up to date especially when important bugs have been fixed.

@kirklund kirklund marked this pull request as draft July 21, 2020 21:32
@kirklund
Copy link
Contributor Author

I don't have time for this PR.

@kirklund kirklund closed this Aug 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants