[SPARK-11093] [Core] ChildFirstURLClassLoader#getResources should return all found resources, not just those in the child classloader

[alewando]

No description provided.

[vanzin]

ok to test

[SparkQA]

Test build #43694 has finished for PR 9106 at commit 87cf157.

• This patch fails MiMa tests.
• This patch merges cleanly.
• This patch adds no public classes.

[alewando]

retest this please

[srowen]

I get this, and it makes sense, but in the case you describe, the child classloader had no resource under this name. It seems like it would have already returned the parent's copy. Is this really the nature of the problem then or do I misunderstand?

[alewando]

In my situation, the child classloader did have resources under this name. With the Typesafe config library, it is typical (and suggested) that libraries package their own default configurations in a 'reference.conf' file at the root of their jar, and that applications do the same. All of the found configuration files are merged together. So in my case, the application assembly (in the child classloader) has it's own 'reference.conf' resource, which (due to this issue) is preventing akka's 'reference.conf' resource (in the parent classloader) from being seen.

[srowen]

I get it. This makes sense, since the general contract is to return all visible resources under the name.

[SparkQA]

Test build #1897 has finished for PR 9106 at commit 87cf157.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[vanzin]

nit: these imports should come before org.apache.spark. Also, nuke one of the empty lines before the class declaration.

[vanzin]

LGTM, aside from minor style nit.

[alewando]

Fixed

[alewando]

2nd time on this PR for test failure due to (seemingly) network issues

[alewando]

retest this please

[vanzin]

retest this please

[SparkQA]

Test build #43728 has finished for PR 9106 at commit d0aa6aa.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[vanzin]

ah, missed this before, but tests should use ===. Other than that, patch looks good.

[vanzin]

retest this please

[SparkQA]

Test build #43752 has finished for PR 9106 at commit 8f1b6d9.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #43754 has finished for PR 9106 at commit 8f1b6d9.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[vanzin]

Merging to master

[alewando]

Closed but not merged?

[srowen]

@alewando no it was merged; see above 0f62c22