LUTECE-2196 : Allow to reload the log4j configuration at runtime#140
LUTECE-2196 : Allow to reload the log4j configuration at runtime#140jonenst wants to merge 1 commit intodevelop7.xfrom
Conversation
| manage_caches.actionViewKeys=View config and keys | ||
| manage_caches.buttonFlush=Flush all | ||
| manage_caches.buttonViewKeys=View config and keys | ||
| manage_caches.titleReloadLogsConfiguration=Log configuration loading |
There was a problem hiding this comment.
I chose the same expression as the other "reloading" button which says "Properties Loading". I don't have a strong opinion
|
|
||
| // Conf | ||
| private static final String PATH_CONF = "/WEB-INF/conf/"; | ||
| private static final String FILE_PROPERTIES_CONFIG = "config.properties"; |
There was a problem hiding this comment.
Maybe you shoud not redefine these constants here, but share them with the normal intialisation process. Otherwise, there is a risk these become desynchronized
There was a problem hiding this comment.
It's already defined in several places as private constants... Not sure if it's useful to declare them as public constants. Maybe in fr.paris.lutece.portal.web.constants.Parameters ? but it looks like this class is for parameters of http requests.
|
|
||
| return JSP_MANAGE_CACHES; | ||
| } | ||
|
|
There was a problem hiding this comment.
You should be able to add tests that
- ensure the CSRF protection is enforced
- verify that the log configuration is indeed changed
| </@tform> | ||
| <@tform method='post' action='jsp/admin/system/DoReloadLogsConfiguration.jsp' class='pull-right spaced'> | ||
| <input type="hidden" name="token" value="${token}"> | ||
| <@button type='submit' buttonIcon='refresh' title='#i18n{portal.system.manage_caches.titleReloadLogsConfiguration}' id='reloadLogsConf' size='' showTitleXs=false showTitleSm=false /> |
There was a problem hiding this comment.
So the button shows no text on small screens ? Since there are no tooltips on smartphone, the purpose of this button might not be obvious
There was a problem hiding this comment.
If we go with having this button on this page, then maybe we can use a single letter to disambiguate ? L and P. Or Logs and Props ? Or Logs and Properties ?
If we put this functionality in another page maybe the "reload" glyph is enough
There was a problem hiding this comment.
why not just display the title at all sizes ?
|
Thanks for the review, I'll fix the problems you found |
seboo
force-pushed
the
develop
branch
3 times, most recently
from
135e4f9 to
e399c1d
Compare
2 participants
We should discuss where we want this feature