You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lenya.apache.org by Andreas Hartmann <an...@apache.org> on 2006/03/13 17:43:19 UTC
Logging issues (was: svn commit: r385571 - /lenya/trunk/src/java/org/apache/lenya/cms/publication/DefaultDocument.java)
Hi Michi,
thanks for your commits - but I have some comments:
> +import org.apache.log4j.Category;
please don't introduce a custom logging concept for single classes.
DefaultDocument is already LogEnabled.
>
> /**
> * A typical CMS document.
> @@ -42,6 +43,8 @@
> */
> public class DefaultDocument extends AbstractLogEnabled implements Document {
>
> + private Category log = Category.getInstance(DefaultDocument.class);
> +
> private DocumentIdentifier identifier;
> private String sourceURI;
> private DocumentIdentityMap identityMap;
> @@ -134,11 +137,15 @@
> List documentLanguages = new ArrayList();
> String[] allLanguages = getPublication().getLanguages();
>
> + log.debug("Number of languages of this publication: " + allLanguages.length);
Please use
if (getLogger().isDebugEnabled()) { ... }
for performance reasons.
> +
> for (int i = 0; i < allLanguages.length; i++) {
> Document version;
> try {
> + log.debug("Try to create document: " + allLanguages[i] + " " + this);
> version = getIdentityMap().getLanguageVersion(this, allLanguages[i]);
> } catch (DocumentBuildException e) {
> + log.warn(e.getMessage());
> throw new DocumentException(e);
> }
> if (version.exists()) {
> @@ -191,6 +198,9 @@
> return this.extension;
> }
>
> + /**
> + * @see org.apache.lenya.cms.publication.Document#getSourceExtension()
> + */
> public String getSourceExtension() {
> String extension;
> try {
> @@ -200,6 +210,7 @@
> throw new RuntimeException(e);
> }
> if (extension == null) {
> + getLogger().warn("No source extension. The extension \"xml\" will be used as default!");
> extension = "xml";
> }
> return extension;
> @@ -264,6 +275,20 @@
> public boolean existsInAnyLanguage() throws DocumentException {
> boolean exists = false;
>
> + String[] languages = getLanguages();
> +
> + if (languages.length > 0) {
> + log.warn("Document (" + this + ") exists in at least one language: " + languages.length);
This warning message is not appropriate. Please use debug instead.
Too many warning logs will affect performance.
> + String[] allLanguages = getPublication().getLanguages();
> + if (languages.length == allLanguages.length) log.warn("Document (" + this + ") exists even in all languages of this publication");
> + return true;
> + } else {
> + log.warn("Document (" + this + ") does NOT exist in any language");
Same here. A warning is not appropriate here, because it is normal
behaviour.
-- Andreas
> + return false;
> + }
> +
> +// NOTE: This seems to be unecessary because getLanguage() already creates these documents
> +/*
> try {
> String[] languages = getLanguages();
> for (int i = 0; i < languages.length; i++) {
> @@ -274,6 +299,7 @@
> throw new DocumentException(e);
> }
> return exists;
> +*/
> }
>
> protected DocumentIdentifier getIdentifier() {
> @@ -341,16 +367,22 @@
> * @see org.apache.lenya.cms.metadata.MetaDataOwner#getMetaDataManager()
> */
> public MetaDataManager getMetaDataManager() {
> + log.debug("getSourceURI(): " + getPublication().getSourceURI());
> if (this.metaDataManager == null) {
> SourceResolver resolver = null;
> RepositorySource source = null;
> try {
> resolver = (SourceResolver) this.manager.lookup(SourceResolver.ROLE);
> +
> + // TODO: This needs to be improved ...
> String sourceUri = getPublication().getSourceURI() + "/content/" + getArea()
> + getId() + "/index_" + getLanguage() + ".xml";
> +
> + log.debug("Source URI: " + sourceUri);
> source = (RepositorySource) resolver.resolveURI(sourceUri);
> this.metaDataManager = source.getNode().getMetaDataManager();
> } catch (Exception e) {
> + log.warn(e.getMessage());
> throw new RuntimeException(e);
> } finally {
> if (resolver != null) {
--
Andreas Hartmann
Wyona Inc. - Open Source Content Management - Apache Lenya
http://www.wyona.com http://lenya.apache.org
andreas.hartmann@wyona.com andreas@apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org
Re: Logging issues
Posted by Michael Wechner <mi...@wyona.com>.
Andreas Hartmann wrote:
> Michael Wechner wrote:
>
> [...]
>
>>>> the problem I see is that the DefaultDocument is an implementation
>>>> and if the implementation
>>>> itself does not throw a WARNING or an ERROR, but only higher up,
>>>> then it's very hard to debug
>>>> where the actually problem is located.
>>>
>>>
>>>
>>> I don't understand this implication. You throw an exception when a
>>> problem occurs. Why should you log a warning if the caller might
>>> interpret the result of this method as a problem? Then you could
>>> log warnings in virtually every line of the code.
>>
>>
>>
>> I am not talking about logging warnings for every line, but for
>> particular cases.
>
>
> You're suggesting to log warning in existsInAnyLanguage().
>
> Some cases where this method could be used:
>
> - preconditions of "create new document" (check if document already
> exists)
> - moving documents (check if target document already exists)
> - searching for documents by document ID
> - checking for dead links
> - ...
>
> In none of these cases, a "potentially harmful situation" occurs.
what do you mean by "harmful"? The system might not crash just because a
document does not exist,
but it's nevertheless important to know why it doesn't exist, because
the system is worthless if doesn't
do what it is intended for. A car which makes a sound is great, but if
it doesn't drive what good is it.
This is why I think WARNINGS are appropriate.
>
>
>> Lenya currently sends you a message in the sense "no such document"
>> which really sucks,
>> because it gives you no clue why there is no such document.
>
>
> How is this related to logging?
> I agree that the error handling could be improved, but I don't
> see the connection between warning logs and meaningless error screens ...
at least I might find something useful within the logfiles. At the
moment I cannot find
anything really useful within the logfile or it's very hard to figure
hard what might be the
actual problem (see also Josias notes re "avalon keys")
>
>
>> Is it a problem of the particular SiteManager? is it a problem of the
>> particular Repository?
>> Is it a problem of the SourceImplementation?
>>
>> In the case of more complex resource types this can become even more
>> annoying ...
>>
>> I have spent a lot of time over the last couple of days to figure out
>> current bugs and
>> just because the logging sucks. Just wait until you will spend time
>> on this ;-)
>
>
> I guess I misunderstand this sentence - it seems to imply that
> I have not spent any time fixing bugs in 1.4-dev ...
no, not all. But it seems that it's very simple for you to debug stuff.
So either I don't know something which you know, or I am just stupid.
Why not log stuff which really helps in the case of a problem. Why make
people's life
difficult?
>
>
>> Also consider that in productive environments it will be even more a
>> problem.
>>
>> Or do you really want to end up with something like:
>>
>>
>> "Unexpected System Failure" (go ask your admin)
>>
>> ?
>
>
> Why do you ask me that? Did I somehow suggest that?
no, but currently that's how Lenya works and I want to fix this.
And I appreciate any help on this.
Michi
>
> -- Andreas
>
>
--
Michael Wechner
Wyona - Open Source Content Management - Apache Lenya
http://www.wyona.com http://lenya.apache.org
michael.wechner@wyona.com michi@apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org
Re: Logging issues
Posted by Andreas Hartmann <an...@apache.org>.
Michael Wechner wrote:
[...]
>>> the problem I see is that the DefaultDocument is an implementation
>>> and if the implementation
>>> itself does not throw a WARNING or an ERROR, but only higher up, then
>>> it's very hard to debug
>>> where the actually problem is located.
>>
>>
>> I don't understand this implication. You throw an exception when a
>> problem occurs. Why should you log a warning if the caller might
>> interpret the result of this method as a problem? Then you could
>> log warnings in virtually every line of the code.
>
>
> I am not talking about logging warnings for every line, but for
> particular cases.
You're suggesting to log warning in existsInAnyLanguage().
Some cases where this method could be used:
- preconditions of "create new document" (check if document already exists)
- moving documents (check if target document already exists)
- searching for documents by document ID
- checking for dead links
- ...
In none of these cases, a "potentially harmful situation" occurs.
> Lenya currently sends you a message in the sense "no such document"
> which really sucks,
> because it gives you no clue why there is no such document.
How is this related to logging?
I agree that the error handling could be improved, but I don't
see the connection between warning logs and meaningless error screens ...
> Is it a problem of the particular SiteManager? is it a problem of the
> particular Repository?
> Is it a problem of the SourceImplementation?
>
> In the case of more complex resource types this can become even more
> annoying ...
>
> I have spent a lot of time over the last couple of days to figure out
> current bugs and
> just because the logging sucks. Just wait until you will spend time on
> this ;-)
I guess I misunderstand this sentence - it seems to imply that
I have not spent any time fixing bugs in 1.4-dev ...
> Also consider that in productive environments it will be even more a
> problem.
>
> Or do you really want to end up with something like:
>
>
> "Unexpected System Failure" (go ask your admin)
>
> ?
Why do you ask me that? Did I somehow suggest that?
-- Andreas
--
Andreas Hartmann
Wyona Inc. - Open Source Content Management - Apache Lenya
http://www.wyona.com http://lenya.apache.org
andreas.hartmann@wyona.com andreas@apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org
Re: Logging issues
Posted by Michael Wechner <mi...@wyona.com>.
Andreas Hartmann wrote:
> Michael Wechner wrote:
>
> [...]
>
>>> Do you think it's a "potentially harmful situation" if
>>> the method
>>> existsInAnyLanguage() returns false?
>>
>>
>> false, there is no language version and I think this should be
>> indicated by at least a WARNING
>
>
> No, that's not the case. The check for the existing language version
> implicates that the caller is aware of the case that the language
> version does not exist. This method could be called, for instance, to
> enable / disable the "Create language version" menu item.
>
>
>>> IMHO, only the caller of this method can determine whether the result
>>> (true or false) is harmful for the application. For the method itself,
>>> either result is fine.
>>
>
> I totally agree to Josias.
>
>
>> the problem I see is that the DefaultDocument is an implementation
>> and if the implementation
>> itself does not throw a WARNING or an ERROR, but only higher up, then
>> it's very hard to debug
>> where the actually problem is located.
>
>
> I don't understand this implication. You throw an exception when a
> problem occurs. Why should you log a warning if the caller might
> interpret the result of this method as a problem? Then you could
> log warnings in virtually every line of the code.
I am not talking about logging warnings for every line, but for
particular cases.
Lenya currently sends you a message in the sense "no such document"
which really sucks,
because it gives you no clue why there is no such document.
Is it a problem of the particular SiteManager? is it a problem of the
particular Repository?
Is it a problem of the SourceImplementation?
In the case of more complex resource types this can become even more
annoying ...
I have spent a lot of time over the last couple of days to figure out
current bugs and
just because the logging sucks. Just wait until you will spend time on
this ;-)
Also consider that in productive environments it will be even more a
problem.
Or do you really want to end up with something like:
"Unexpected System Failure" (go ask your admin)
?
I don't and I don't understand at all why a lot of software has such
crappy behaviour.
I think we should make Lenya a good exception to this rule of bad behaviour.
Michi
>
>
>
>> If one would throw a stacktrace then it should be alright,
>> but so far we don't log stacktraces for WARNINGS.
>
>
> If an exception is thrown, a stack trace is logged.
>
> -- Andreas
>
>
--
Michael Wechner
Wyona - Open Source Content Management - Apache Lenya
http://www.wyona.com http://lenya.apache.org
michael.wechner@wyona.com michi@apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org
Re: Logging issues
Posted by Andreas Hartmann <an...@apache.org>.
Michael Wechner wrote:
[...]
>> Do you think it's a "potentially harmful situation" if
>> the method
>> existsInAnyLanguage() returns false?
>>
>
> false, there is no language version and I think this should be indicated
> by at least a WARNING
No, that's not the case. The check for the existing language version
implicates that the caller is aware of the case that the language
version does not exist. This method could be called, for instance, to
enable / disable the "Create language version" menu item.
>> IMHO, only the caller of this method can determine whether the result
>> (true or false) is harmful for the application. For the method itself,
>> either result is fine.
I totally agree to Josias.
> the problem I see is that the DefaultDocument is an implementation and
> if the implementation
> itself does not throw a WARNING or an ERROR, but only higher up, then
> it's very hard to debug
> where the actually problem is located.
I don't understand this implication. You throw an exception when a
problem occurs. Why should you log a warning if the caller might
interpret the result of this method as a problem? Then you could
log warnings in virtually every line of the code.
> If one would throw a stacktrace
> then it should be alright,
> but so far we don't log stacktraces for WARNINGS.
If an exception is thrown, a stack trace is logged.
-- Andreas
--
Andreas Hartmann
Wyona Inc. - Open Source Content Management - Apache Lenya
http://www.wyona.com http://lenya.apache.org
andreas.hartmann@wyona.com andreas@apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org
Re: Logging issues
Posted by Michael Wechner <mi...@wyona.com>.
Josias Thöny wrote:
>On Mon, 2006-03-13 at 19:46 +0100, Michael Wechner wrote:
>
>
>>Andreas Hartmann wrote:
>>
>>
>>
>>>Hi Michi,
>>>
>>>thanks for your commits - but I have some comments:
>>>
>>>
>>>
>>>>+import org.apache.log4j.Category;
>>>>
>>>>
>>>please don't introduce a custom logging concept for single classes.
>>>DefaultDocument is already LogEnabled.
>>>
>>>
>>I know, but the current logging concept really sucks if I might say so.
>>If you are able to show how one can configure it in a better way, then I
>>am fine,
>>but the logging information as it is just doesn't help at all.
>>
>>
>
>What I don't understand about the current logging is the logger names.
>For example, the output of PublicationImpl belongs to the logger
>"core.manager", and the output of DefaultDocument belongs to
>"lenya.publication" (at least that's what I see in log4j.log).
>
>It seems the names are associated to the avalon components, but it makes
>it kinda difficult to filter the log output of a certain java class.
>
>
I think so too. If one uses log4j directly and uses the following
pattern within log4j.xconf
<param name="ConversionPattern" value="%-4r %d [%t] %-5p %c.%M():%L %x -
%m%n"/>
then debugging will become very easy, because one will know exactly what
class, method and line ...
>
>
>Do you think it's a "potentially harmful situation" if the method
>existsInAnyLanguage() returns false?
>
>
false, there is no language version and I think this should be indicated
by at least a WARNING
>IMHO, only the caller of this method can determine whether the result
>(true or false) is harmful for the application. For the method itself,
>either result is fine.
>
>
the problem I see is that the DefaultDocument is an implementation and
if the implementation
itself does not throw a WARNING or an ERROR, but only higher up, then
it's very hard to debug
where the actually problem is located. If one would throw a stacktrace
then it should be alright,
but so far we don't log stacktraces for WARNINGS.
Michi
>Josias
>
>[1] http://logging.apache.org/log4j/docs/api/org/apache/log4j/Level.html
>
>
>
>
>>Michi
>>
>>
>>
>>>-- Andreas
>>>
>>>
>>>
>>>
>>>>+ return false;
>>>>+ }
>>>>+
>>>>+// NOTE: This seems to be unecessary because getLanguage() already
>>>>creates these documents
>>>>+/*
>>>> try {
>>>> String[] languages = getLanguages();
>>>> for (int i = 0; i < languages.length; i++) {
>>>>@@ -274,6 +299,7 @@
>>>> throw new DocumentException(e);
>>>> }
>>>> return exists;
>>>>+*/
>>>> }
>>>>
>>>> protected DocumentIdentifier getIdentifier() {
>>>>@@ -341,16 +367,22 @@
>>>> * @see
>>>>org.apache.lenya.cms.metadata.MetaDataOwner#getMetaDataManager()
>>>> */
>>>> public MetaDataManager getMetaDataManager() {
>>>>+ log.debug("getSourceURI(): " +
>>>>getPublication().getSourceURI());
>>>> if (this.metaDataManager == null) {
>>>> SourceResolver resolver = null;
>>>> RepositorySource source = null;
>>>> try {
>>>> resolver = (SourceResolver)
>>>>this.manager.lookup(SourceResolver.ROLE);
>>>>+
>>>>+ // TODO: This needs to be improved ...
>>>> String sourceUri = getPublication().getSourceURI() +
>>>>"/content/" + getArea()
>>>> + getId() + "/index_" + getLanguage() + ".xml";
>>>>+
>>>>+ log.debug("Source URI: " + sourceUri);
>>>> source = (RepositorySource)
>>>>resolver.resolveURI(sourceUri);
>>>> this.metaDataManager =
>>>>source.getNode().getMetaDataManager();
>>>> } catch (Exception e) {
>>>>+ log.warn(e.getMessage());
>>>> throw new RuntimeException(e);
>>>> } finally {
>>>> if (resolver != null) {
>>>>
>>>>
>>>
>>>
>>>
>>
>>
>
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
>For additional commands, e-mail: dev-help@lenya.apache.org
>
>
>
>
--
Michael Wechner
Wyona - Open Source Content Management - Apache Lenya
http://www.wyona.com http://lenya.apache.org
michael.wechner@wyona.com michi@apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org
Re: Logging issues
Posted by Andreas Hartmann <an...@apache.org>.
Josias Thöny wrote:
> On Mon, 2006-03-13 at 19:46 +0100, Michael Wechner wrote:
>> Andreas Hartmann wrote:
>>
>>> Hi Michi,
>>>
>>> thanks for your commits - but I have some comments:
>>>
>>>> +import org.apache.log4j.Category;
>>>
>>> please don't introduce a custom logging concept for single classes.
>>> DefaultDocument is already LogEnabled.
>>
>> I know, but the current logging concept really sucks if I might say so.
>> If you are able to show how one can configure it in a better way, then I
>> am fine,
>> but the logging information as it is just doesn't help at all.
>
> What I don't understand about the current logging is the logger names.
> For example, the output of PublicationImpl belongs to the logger
> "core.manager",
That's because the PublicationManager component isn't configured
in cocoon.xconf. You could add for instance
<component class="org.apache.lenya.cms.publication.PublicationManagerImpl"
role="org.apache.lenya.cms.publication.PublicationManager"
logger="lenya.publication"/>
Then the logger "lenya.publication" is used.
> and the output of DefaultDocument belongs to
> "lenya.publication" (at least that's what I see in log4j.log).
The DefaultDocumentBuilder has the lenya.publication logger:
<document-builders>
<component-instance
class="org.apache.lenya.cms.publication.DefaultDocumentBuilder"
logger="lenya.publication" name="default"/>
</document-builders>
> It seems the names are associated to the avalon components, but it makes
> it kinda difficult to filter the log output of a certain java class.
If we need finer logging granularity, we could replace
ContainerUtil.enableLogging(publication, getLogger());
with
ContainerUtil.enableLogging(publication, getLogger().getChildLogger(...));
IMO component-level granularity is sufficient in most cases, but
it's fine with me to change it.
-- Andreas
--
Andreas Hartmann
Wyona Inc. - Open Source Content Management - Apache Lenya
http://www.wyona.com http://lenya.apache.org
andreas.hartmann@wyona.com andreas@apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org
Re: Logging issues
Posted by Josias Thöny <jo...@wyona.com>.
On Mon, 2006-03-13 at 19:46 +0100, Michael Wechner wrote:
> Andreas Hartmann wrote:
>
> >
> > Hi Michi,
> >
> > thanks for your commits - but I have some comments:
> >
> >> +import org.apache.log4j.Category;
> >
> >
> > please don't introduce a custom logging concept for single classes.
> > DefaultDocument is already LogEnabled.
>
>
> I know, but the current logging concept really sucks if I might say so.
> If you are able to show how one can configure it in a better way, then I
> am fine,
> but the logging information as it is just doesn't help at all.
What I don't understand about the current logging is the logger names.
For example, the output of PublicationImpl belongs to the logger
"core.manager", and the output of DefaultDocument belongs to
"lenya.publication" (at least that's what I see in log4j.log).
It seems the names are associated to the avalon components, but it makes
it kinda difficult to filter the log output of a certain java class.
>
> >
> >>
> >> +
> >> + if (languages.length > 0) {
> >> + log.warn("Document (" + this + ") exists in at least
> >> one language: " + languages.length);
> >
> >
> > This warning message is not appropriate. Please use debug instead.
>
>
> agreed
>
> > Too many warning logs will affect performance.
> >
> >
> >> + String[] allLanguages =
> >> getPublication().getLanguages();
> >> + if (languages.length == allLanguages.length)
> >> log.warn("Document (" + this + ") exists even in all languages of
> >> this publication");
> >> + return true;
> >> + } else {
> >> + log.warn("Document (" + this + ") does NOT exist in
> >> any language");
> >
> >
> > Same here. A warning is not appropriate here, because it is normal
> > behaviour.
>
>
> Even if it's normal behaviour I think a WARNING makes perfect sense,
> because no language exists
> and I think it makes a lot of sense to tell where it is being determined
> to figure out what might
> be the reason for this
FYI, these are the descriptions of the log levels from the log4j docu,
see [1]:
OFF
The OFF has the highest possible rank and is intended to turn
off logging.
________________________________________________________________________
FATAL
The FATAL level designates very severe error events that will
presumably lead the application to abort.
________________________________________________________________________
ERROR
The ERROR level designates error events that might still allow
the application to continue running.
________________________________________________________________________
WARN
The WARN level designates potentially harmful situations.
________________________________________________________________________
INFO
The INFO level designates informational messages that highlight
the progress of the application at coarse-grained level.
________________________________________________________________________
DEBUG
The DEBUG Level designates fine-grained informational events
that are most useful to debug an application.
________________________________________________________________________
TRACE
The TRACE Level designates finer-grained informational events
than the DEBUG
________________________________________________________________________
ALL
The ALL has the lowest possible rank and is intended to turn on
all logging.
Do you think it's a "potentially harmful situation" if the method
existsInAnyLanguage() returns false?
IMHO, only the caller of this method can determine whether the result
(true or false) is harmful for the application. For the method itself,
either result is fine.
Josias
[1] http://logging.apache.org/log4j/docs/api/org/apache/log4j/Level.html
>
> Michi
>
> >
> >
> > -- Andreas
> >
> >
> >> + return false;
> >> + }
> >> +
> >> +// NOTE: This seems to be unecessary because getLanguage() already
> >> creates these documents
> >> +/*
> >> try {
> >> String[] languages = getLanguages();
> >> for (int i = 0; i < languages.length; i++) {
> >> @@ -274,6 +299,7 @@
> >> throw new DocumentException(e);
> >> }
> >> return exists;
> >> +*/
> >> }
> >>
> >> protected DocumentIdentifier getIdentifier() {
> >> @@ -341,16 +367,22 @@
> >> * @see
> >> org.apache.lenya.cms.metadata.MetaDataOwner#getMetaDataManager()
> >> */
> >> public MetaDataManager getMetaDataManager() {
> >> + log.debug("getSourceURI(): " +
> >> getPublication().getSourceURI());
> >> if (this.metaDataManager == null) {
> >> SourceResolver resolver = null;
> >> RepositorySource source = null;
> >> try {
> >> resolver = (SourceResolver)
> >> this.manager.lookup(SourceResolver.ROLE);
> >> +
> >> + // TODO: This needs to be improved ...
> >> String sourceUri = getPublication().getSourceURI() +
> >> "/content/" + getArea()
> >> + getId() + "/index_" + getLanguage() + ".xml";
> >> +
> >> + log.debug("Source URI: " + sourceUri);
> >> source = (RepositorySource)
> >> resolver.resolveURI(sourceUri);
> >> this.metaDataManager =
> >> source.getNode().getMetaDataManager();
> >> } catch (Exception e) {
> >> + log.warn(e.getMessage());
> >> throw new RuntimeException(e);
> >> } finally {
> >> if (resolver != null) {
> >
> >
> >
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org
Re: Logging issues
Posted by Michael Wechner <mi...@wyona.com>.
Andreas Hartmann wrote:
>
> Hi Michi,
>
> thanks for your commits - but I have some comments:
>
>> +import org.apache.log4j.Category;
>
>
> please don't introduce a custom logging concept for single classes.
> DefaultDocument is already LogEnabled.
I know, but the current logging concept really sucks if I might say so.
If you are able to show how one can configure it in a better way, then I
am fine,
but the logging information as it is just doesn't help at all.
>
>>
>> +
>> + if (languages.length > 0) {
>> + log.warn("Document (" + this + ") exists in at least
>> one language: " + languages.length);
>
>
> This warning message is not appropriate. Please use debug instead.
agreed
> Too many warning logs will affect performance.
>
>
>> + String[] allLanguages =
>> getPublication().getLanguages();
>> + if (languages.length == allLanguages.length)
>> log.warn("Document (" + this + ") exists even in all languages of
>> this publication");
>> + return true;
>> + } else {
>> + log.warn("Document (" + this + ") does NOT exist in
>> any language");
>
>
> Same here. A warning is not appropriate here, because it is normal
> behaviour.
Even if it's normal behaviour I think a WARNING makes perfect sense,
because no language exists
and I think it makes a lot of sense to tell where it is being determined
to figure out what might
be the reason for this
Michi
>
>
> -- Andreas
>
>
>> + return false;
>> + }
>> +
>> +// NOTE: This seems to be unecessary because getLanguage() already
>> creates these documents
>> +/*
>> try {
>> String[] languages = getLanguages();
>> for (int i = 0; i < languages.length; i++) {
>> @@ -274,6 +299,7 @@
>> throw new DocumentException(e);
>> }
>> return exists;
>> +*/
>> }
>>
>> protected DocumentIdentifier getIdentifier() {
>> @@ -341,16 +367,22 @@
>> * @see
>> org.apache.lenya.cms.metadata.MetaDataOwner#getMetaDataManager()
>> */
>> public MetaDataManager getMetaDataManager() {
>> + log.debug("getSourceURI(): " +
>> getPublication().getSourceURI());
>> if (this.metaDataManager == null) {
>> SourceResolver resolver = null;
>> RepositorySource source = null;
>> try {
>> resolver = (SourceResolver)
>> this.manager.lookup(SourceResolver.ROLE);
>> +
>> + // TODO: This needs to be improved ...
>> String sourceUri = getPublication().getSourceURI() +
>> "/content/" + getArea()
>> + getId() + "/index_" + getLanguage() + ".xml";
>> +
>> + log.debug("Source URI: " + sourceUri);
>> source = (RepositorySource)
>> resolver.resolveURI(sourceUri);
>> this.metaDataManager =
>> source.getNode().getMetaDataManager();
>> } catch (Exception e) {
>> + log.warn(e.getMessage());
>> throw new RuntimeException(e);
>> } finally {
>> if (resolver != null) {
>
>
>
--
Michael Wechner
Wyona - Open Source Content Management - Apache Lenya
http://www.wyona.com http://lenya.apache.org
michael.wechner@wyona.com michi@apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org