db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Lily Wei (JIRA)" <j...@apache.org>
Subject [jira] Updated: (DERBY-4715) Write jvm information and path of derby.jar to derby.log
Date Mon, 12 Jul 2010 21:12:50 GMT

     [ https://issues.apache.org/jira/browse/DERBY-4715?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Lily Wei updated DERBY-4715:
----------------------------

    Attachment: DERBY-4715-4.diff

Thanks Knut, Kathey and Kristian for all the commets.
I made the following changes and attach DERBY-4715-4.diff patch for review.

Comments:
- The methods jarClassPath() and buildJvmVersion() have different indentation than the other
methods. 
Changes according to the current file indentation. Right now, the method starts at space 5
while the context starts at space 9. 

- If formatURL() gets an IOException, it returns the string "IOException". Perhaps it should
also return the exception's message text? 
It will be return message text for IOException now.

- I don't follow all the logic in formatURL(). Could you add some comments that explain the
various transformations? In my environment, just printing what URL.toString() returns gives
clear enough information: 

file:/code/derby/trunk0/classes/ or 
file:/code/derby/trunk0/jars/sane/derby.jar 
Great point. I always believe coding less is better. Use URL.toString() in the code.

- For readability, I think it would be better to separate the properties with a newline character
instead of a space. 
The property is separated by a newline instead of a space.

- The comments in the policy files say "Add for DERBY-4715". I think it would be better if
they said why the permission was needed since we include them as templates in the distribution
and refer to them from the documentation[1], so they are expected to be read by users. Perhaps
something like "getProtectionDomain is an optional permission needed for printing classpath
information to derby.log." 
All the comment relate to java.lang.RuntimePermission is using this comment.

- In server.policy and template.policy, the new permission is added in the section that is
labeled "These permissions are needed for everyday, embedded Derby usage." My understanding
of the patch is that the new permission is optional, and embedded Derby will work fine without
it, it just won't print the classpath to derby.log. If it's so, I'd suggest moving it to a
less prominent section of the policy files. 
Put the java.lang.RuntimePermission "getProtectionDomain" and the comment in its own section

- Similarly, in AssertFailureTest.policy, the permission is added under the label "These are
the ones that matter", but AssertFailureTest runs fine without it.
Make similar change as above and place in less prominent section.

Regarding the public methods in utility classes, there are two new methods - jarClassPatha
and buildJvmVersion after removing formatUrl() in BaseDataFileFactory.java. They are not so
generic to be used or provide a lot of value in the utility classes.

I am running tests against this patch now.


> Write jvm information and path of derby.jar to derby.log
> --------------------------------------------------------
>
>                 Key: DERBY-4715
>                 URL: https://issues.apache.org/jira/browse/DERBY-4715
>             Project: Derby
>          Issue Type: Bug
>          Components: Miscellaneous
>    Affects Versions: 10.6.1.0
>            Reporter: Lily Wei
>            Assignee: Lily Wei
>            Priority: Minor
>             Fix For: 10.7.0.0
>
>         Attachments: DERBY-4715-1.diff, DERBY-4715-2.diff, DERBY-4715-3.diff, DERBY-4715-4.diff,
derby.log
>
>
> The bug is part of DERBY-1272. In production environment, derby.jar can be located different
than the derbyclient.jar It can be easier if we have jvm version information and path of derby.jar
are in the derby.log

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message