Log resolving errors from LibraryClasspathContainerResolverService #3180
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #3180 +/- ##
============================================
+ Coverage 69.75% 69.87% +0.11%
- Complexity 2931 2939 +8
============================================
Files 372 372
Lines 13422 13424 +2
Branches 1611 1612 +1
============================================
+ Hits 9363 9380 +17
+ Misses 3377 3365 -12
+ Partials 682 679 -3
Continue to review full report at Codecov.
|
chanseokoh
previously approved these changes
Jun 26, 2018
|
I'll commit this once I have a patch for #3181. |
|
Hmm, now the logs are littered with exceptions (below) as the fix for #3181 is now executing the remainder of The first type of exception is because we don't have source for Our SourceAttachers then update the container state with the source bindings, which is failing as the test has completed in the meantime and deleted the project. These SourceAttacherJobs should be part of a family so that |
briandealwis
force-pushed
the
logresolutionerrors
branch
from
788ed4e
to
26e0f7b
Compare
elharo
reviewed
Jul 2, 2018
| serializer.saveContainer(javaProject, container); | ||
| for (Job job : sourceAttacherJobs) { | ||
| job.schedule(); | ||
| if (!javaProject.getProject().getWorkspace().isTreeLocked()) { |
There was a problem hiding this comment.
This looks like the PR is doing more than simply logging errors.
elharo
reviewed
Jul 16, 2018
|
It turned into a bit of a tar-baby. The trigger (failing downloads leaving And then there's a separate issue of forking off separate source-jar resolvers. I'm not sure it actually buys us that much since on completion the jobs update the classpath, which will trigger a new build. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I was hitting puzzling errors running the
ImportNativeAppEngineStandardProjectTestintegration tests locally where our classpath container had no entries. It was really puzzling! I discovered that ourLibraryClasspathContainerResolverServicewasn't logging a key exception, which clued me in that Maven had failed to download an artifact and left a little.lastUpdateddropping that prevents Maven from attempting to re-resolve for some amount of time.This patch causes that exception to be logged.
Fixes #3181 too.