You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by GitBox <gi...@apache.org> on 2021/03/17 18:48:47 UTC

[GitHub] [tomcat] rmannibucau opened a new pull request #414: don't log undesired warnings for webapp classloader configuration

rmannibucau opened a new pull request #414:
URL: https://github.com/apache/tomcat/pull/414


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] rmaucher closed pull request #414: don't log undesired warnings for webapp classloader configuration

Posted by GitBox <gi...@apache.org>.
rmaucher closed pull request #414:
URL: https://github.com/apache/tomcat/pull/414


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] rmaucher commented on pull request #414: don't log undesired warnings for webapp classloader configuration

Posted by GitBox <gi...@apache.org>.
rmaucher commented on pull request #414:
URL: https://github.com/apache/tomcat/pull/414#issuecomment-801817917


   This was introduced by https://github.com/apache/tomcat/commit/c38e9ccabc0d20e1f88b389ab531dcbf632b0ed8 and I understand the WebappLoader changes part of it.
   
   In StandardContext, I don't see why the properties should not be set simply after using if (getLoader().getClassLoader() instanceof WebappClassLoaderBase) since the StandardContext class already uses WebappLoader (to create a default loader). This would avoid the useless reflection that is causing you problems.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] rmaucher commented on pull request #414: don't log undesired warnings for webapp classloader configuration

Posted by GitBox <gi...@apache.org>.
rmaucher commented on pull request #414:
URL: https://github.com/apache/tomcat/pull/414#issuecomment-801922794


   @rmannibucau That wasn't the case before that commit, but it's possible after it. Personally, I would prefer dropping the reflection. Let's see if Mark wants to comment on that.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] rmaucher commented on pull request #414: don't log undesired warnings for webapp classloader configuration

Posted by GitBox <gi...@apache.org>.
rmaucher commented on pull request #414:
URL: https://github.com/apache/tomcat/pull/414#issuecomment-804012002


   I added the simple reflection free version, so the properties now need WebappCLBase and we'll see what happens. I didn't merge the test since it's quite obvious when there's no reflection and heuristic, but you can add it if you'd like.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] rmannibucau commented on pull request #414: don't log undesired warnings for webapp classloader configuration

Posted by GitBox <gi...@apache.org>.
rmannibucau commented on pull request #414:
URL: https://github.com/apache/tomcat/pull/414#issuecomment-801842654


   @rmaucher due to the code structure I assumed it was intended for custom loader to be able to benefit from the same configuration without having to inherit from the default webapp classloader, if this assumption is true I agree with you it is better to do "if we know then do, else ignore". Not sure how an API it is since it is somehow publicly available.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org