[
https://issues.apache.org/jira/browse/DERBY-1275?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12469264
]
Daniel John Debrunner commented on DERBY-1275:
----------------------------------------------
Patch looks cleaner - thanks. Some minor issue, could be addressed after commit:
- traceDirectory = "." + File.separator + "TraceDir" + File.separator;
I don't think use of '.' for the current directory is portable, I think you
can simply use "TraceDir" here.
- ClientSideSystemPropertiesTest
private static String traceDirectory;
Use of static fields in a JUnit test is not a good practice.
Since the decorator will set the system property, you could just fetch the
value of the system property when you need it.
BaseTestCase has a utility method to do that.
- In the client code the two identical anonymous inner classes could be
combined into one to reduce footprint by having
a private method in ClientBaseDataSource that gets the system property passed
into it.
- You added a method to TestConfiguration - systemPropertiesSetupDecorator.
I guess I didn't understand what value this adds. The decorators that are
returned from static methods
in TestConfiguration are ones that modify the configuration, but where a
decorator can be used directly
it is left as, which I think maps to the typical use of decorators in Junit.
I think maybe the TestConfiguration class has already come too intertwined
with decorators and it might be better to have a Decorator class that just had
static methods. This would at least gather them in a single class so that they
could be easily discovered. I think a Decorator class would be a good place for
the method you added.
> Provide a way to enable client tracing without changing the application
> -----------------------------------------------------------------------
>
> Key: DERBY-1275
> URL: https://issues.apache.org/jira/browse/DERBY-1275
> Project: Derby
> Issue Type: Improvement
> Components: Network Client
> Affects Versions: 10.1.3.1, 10.2.1.6
> Reporter: Kathey Marsden
> Assigned To: Mamta A. Satoor
> Priority: Minor
> Fix For: 10.2.3.0
>
> Attachments: DERBY1275EnableClientTracingDiffV1.txt,
> DERBY1275EnableClientTracingDiffV2.txt,
> DERBY1275EnableClientTracingDiffV3.txt,
> DERBY1275EnableClientTracingStatV1.txt,
> DERBY1275EnableClientTracingStatV2.txt, DERBY1275EnableClientTracingStatV3.txt
>
>
> Currently the client tracing can be enabled by setting attributes on the
> client url, setXXX methods on the DataSource or calling
> DriverManager.setLogWriter(), but it often cannot be enabled in a deployed
> client application because all of these API's require modification of the
> application or its configuration files.
> It would be good to have a global way to turn on client tracing. A system
> property pointing to a property file is one possibility but probably not
> ideal because of the impact in class loader contexts. I am not sure what
> the other possiblities are,
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
