This seems like a very reasonable proposal to me. Allowing users to define System properties via sensor.properties and hackystat.properties can definitely simplify life in certain situations. Some initial thoughts:
* I think it's more maintainable if hackystat code continues to use the SensorProperties accessors to get at properties, rather than just start using System.getProperties, because I think that makes it clearer where the information is supposed to be coming from. (If we limited the properties to java.* etc, that would satisfy this concern.)
* On the other hand, I think there are some advantages to adding all properties rather than having to specify some subset (which we'll be modifying in future as we find new prefixes.) That said, we need to be careful about naming hackystat properties to ensure there's no property clobbering. (We may want to eventually rename properties like HACKYSTAT_HOST to org.hackystat.host. This kind of migration will be much easier once the hackystat installer is available, which could do the property renaming without user intervention).
Any other comments? Unless anyone can see any problems, I propose that I add a Jira issue to do this for the next stable release. It's trivial to implement, of course.
Cheers, Philip
--On Thursday, March 31, 2005 1:18 PM -0500 Todd Olson <[EMAIL PROTECTED]> wrote:
Greetings Hackers,
Proposal: I'd like to propose enhancing SensorProperties and ServerProperties to perform a System.setProperty() (or System.setProperties()).
I propose adding:
System.setProperties(properties)
to each constructor after the respective properties have been read from sensor.properties or hackystat.properties.
Another alternative is to only call System.setProperty() on properties beginning with java.*, javax.*, com.sun.*, etc.
Reasoning: 1. I first encountered this with the "headless" operation. I noticed it is in hackystat.properties but ignored. In order for headless to work, CATALINA_OPTS must be set (mentioned in the build instructions). This does work, but it is nicer to have it all in hackystat.properties.
2. We are working on configuring Hackystat for SSL. We have been successful, but SSL requires a number of System properties to be set. This solution would prevent us from changing a single line of Hackystat code. Plus, it enables the system to support other Java System properties in the future.
We considered add the VM parameters to SensorShell invocations. Unfortunately, setting VM parameters for every call to Sensorshell is quite cumbersome. Sensors live in ANT, VI, Eclipse, etc -- ensuring that every call properly sets the properties is cumbersome.
Of course, we can simply change the code, but I believe that this change is less invasive and is beneficial for future extensibility.
Other ideas/solutions are very welcome!
Thanks, Todd
