-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Issue 554 clean #598
Issue 554 clean #598
Conversation
Change CXF to Spring MVC, that's why most of the java class and configuration had to be updated Use MDC possibilities for CNIL level and filtering authorisation (roles and org) Geotools update made the code slightly change for image generation All in one commit because there were dependencies beetween libraries
add missing space in logback configuration
… at the sametime. Add /services/* mapping in web.xml and not in RequestMapping
…ple specific versionning Remove unused import
Correction on datastoire connection, replacement : in _ not needed anymore Add debug logs to see all available layers name ( to help to see if geo_parcelle not correctly deployed) Change File to byte[], no need to write on disk anymore (save some times)
… "attachement" content-disposition in headers
Add request execution time
Corrections BDD et Demandes Build Docker failed - pb de login docker hub
J'ai juste une question concernant MDC : Est-ce suffisamment fiable pour gérer la sécu d'une application ? De ce que j'ai pu lire ici : https://www.baeldung.com/mdc-in-log4j-2-logback (Partie 6) |
Oui aucun soucis de ce côté, ,il faut bien s'assurer d'avoir les put et remove c'est pour ça que je l'ai mis dans les interceptors. |
Si le thread courant crash (Exception), les remove ne seront pas appliqués. Il faudrait faire un |
Oui bonne idée, je le rajoute, c'est plus propre |
Je suis totalement dépassé ! |
la PR est monstrueuse (réindentation etc) donc difficile a review dans l'état, mais ca build et ca s'installe OK en dev ici. |
Concernant les logs, en utilisant INFO j'ai ça :
On a 2 ligne par requête, pas sur que ce soit nécessaire. |
Je l'ai mis pour pouvoir faire des stats sur les logs avec Kibana après. (Entrée-sortie et du coup indirectement temps de traitement) Mais ça n'a pas une grand plus value effectivement. @MaelREBOUX @landryb vous en pensez quoi je passe la trace en debug ? |
En revanche pour de la stat, je pense que ça serait très utile d'avoir des logs des différentes extractions ou génération qui sont faites du logiciel. Par exemple :
Qui incluraient :
|
Je l'ai fait en mode log debug pour pas impacté les perfs ici en sortant pour le temps de traitement |
Ok je regarderais ça plus en détails. Pour l'instant je fait mes stats d'utilisations cadastrapp depuis les logs du secproxy et ça marche pas trop mal. Pour le reste de ce que j'ai testé ça fonctionne donc OK pour moi pour pousser sur master et taguer comme nouvelle version. |
Je prends ça pour un approved :) |
oui c'etait un approved :) quand je disais monstrueuse, c'est aussi qu'en général c'est bien mieux de faire une PR par fonctionalité avec peu de commits (et donc X PRs), plutot que plein de trucs différents et pas forcément liés les uns aux autres dans la meme PR ;) |
ben moi vu que j'ai accès à rien : ça me fait une belle jambe. Mais sinon oui je pencherai plutôt pour mettre ça en debug pour pas alourdir des log. Il faudrait donc documenter ça ici : https://docs.georchestra.org/cadastrapp/latest/guide_administrateur/configuration_webapp.html Je te fais une issue pour ça ?
Je suis prêt à coucher pour avoir ça dans une table dans la BD... |
Pas forcement très compliqué à mettre en place, il faut juste définir un logger avec ces infos et le brancher sur une BDD. Oui tu peux faire une issue pour garder une trace du besoin. |
Lots of modification in this PR
-> update all libs
-> update geotools management to generate image
-> change CXF to springMVC
-> replace headers information management by input in MDC ( for maintenance purpose )
-> Compatibility with java11
-> add swagger