Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failing to load rome.properties #130

Closed
PatrickGotthard opened this issue Sep 30, 2013 · 8 comments

Comments

@PatrickGotthard
Copy link
Member

commented Sep 30, 2013

=== This issue was migrated from JIRA ===
Type: Bug
Priority: Major
Status: Open
Resolution: UNRESOLVED
Reported by: buckett
Assigned to: ROME Jira Lead
Created: Mon Apr 20 10:47:11 CEST 2009
Updated: Thu Sep 15 20:55:38 CEST 2011
Resolved: Mon Apr 20 13:29:19 CEST 2009
Version: current
Fix version: milestone 1
JIRA Link: https://rometools.jira.com/browse/ROME-129
=========================================

Currently we use an old version of rome, rome-fetcher in Sakai
(http://sakaiproject.org) and I was looking as upgrading them to the latest
version to them. Currently Sakai has complicated classloader setup. There is a
description of it here.

https://source.sakaiproject.org/svn/reference/trunk/docs/architecture/sakai_component.doc

Sakai runs ontop of tomcat and each "component" has it's own classloader that
child of the shared classloader. The API for the component goes in shared so
that both the component and any webapp can access. The component then implements
this APi. But the implementation code can't be accessed by the webapp as the
component's classloader isn't a parent of the webapps. This prevents any webapps
accessing anything in a component.

Currently there is an API for getting RSS feeds which is implemented by a
component containing rome. The problem is that when a webapp calls through to
this component the thread classloader is still set to the webapp's so it
attempts to load the master configuration file (rome.properties) from the
webapp. This fails and causes rome to stop working.

It would be useful if PropertiesLoader could use either the thread classloader
or the classloader that loaded PropertiesLoader to load the configuration.

@PatrickGotthard

This comment has been minimized.

Copy link
Member Author

commented Sep 30, 2013

=== This comment was migrated from JIRA ===
Author: buckett
Created: Mon Apr 20 13:27:45 CEST 2009
===========================================

Created an attachment (id=34)
A patch that makes the classloader rome uses configurable

@PatrickGotthard

This comment has been minimized.

Copy link
Member Author

commented Sep 30, 2013

=== This comment was migrated from JIRA ===
Author: buckett
Created: Mon Apr 20 13:29:19 CEST 2009
===========================================

The attached patch fixes rome so it works in Sakai. It allows the classloader to
be configured through the system property:

rome.use.thread.classloader

It defaults to false which is a change in behaviour from the current version of
rome.

@PatrickGotthard

This comment has been minimized.

Copy link
Member Author

commented Sep 30, 2013

=== This comment was migrated from JIRA ===
Author: anonymous
Created: Thu Sep 15 20:55:38 CEST 2011
===========================================

Attachment rome-129.patch has been added with description: rome-129.patch

@lewismc

This comment has been minimized.

Copy link

commented Jan 16, 2015

Is anyone thinking about testing this patch out?

@sebastian-nagel

This comment has been minimized.

Copy link

commented Jan 27, 2015

This problem is also hit by Apache Nutch (cf. NUTCH-1494 and NUTCH-1893): various documents formats (including feeds) are parsed by plugins, every plugin has its own class loader. The problem is currently worked around by downgrading to rome 0.9.

@buckett

This comment has been minimized.

Copy link
Contributor

commented Sep 16, 2015

Is there anything that needs doing to get this fix into rome? If I created a pull request would that help?

@mishako

This comment has been minimized.

Copy link
Member

commented Dec 20, 2015

@lewismc @sebastian-nagel @buckett
This seems to have been fixed since Rome 1.5.0, i.e. calls to Thread.currentThread().getContextClassLoader() have been removed in 6029420. The class loader used in Rome now is the one that loaded Rome classes and has access to rome.properties.

However you might have problems using Rome in OSGi environment because it's not being packaged as an OSGi bundle. This will be addressed in OSGi-related issues like #143 and #135.

@sebastian-nagel

This comment has been minimized.

Copy link

commented Jan 4, 2016

@mishako @lewismc I can confirm that the problem is fixed when Rome 1.5.1 is used (I didn't test 1.5.0). We are going to upgrade the Rome library dependency, see NUTCH-2193. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.