| Summary: | Unsupported method history | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
| Component: | REST API | Assignee: | David Cook <dcook> |
| Status: | CLOSED FIXED | QA Contact: | |
| Severity: | major | ||
| Priority: | P5 - low | CC: | dcook, lucas, m.de.rooy, martin.renvoize, pedro.amorim, tomascohen |
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | Small patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: |
23.05.00,22.11.04
|
|
| Circulation function: | |||
| Bug Depends on: | 22678 | ||
| Bug Blocks: | |||
| Attachments: |
Bug 33020: Add test
Bug 33020: Add missing Koha::Logger->history Bug 33020: Add test Bug 33020: Add missing Koha::Logger->history Bug 33020: Add test Bug 33020: Add missing Koha::Logger->history Bug 33020: (QA follow-up) POD and chmod |
||
|
Description
Jonathan Druart
2023-02-21 09:38:19 UTC
Caused by commit 490a056f59d0aae52fde84e3450f29b3bc09c87d Bug 22678: Use Koha::Logger for Mojolicious API logger Created attachment 147113 [details] [review] Bug 33020: Add test (In reply to Jonathan Druart from comment #2) > Created attachment 147113 [details] [review] [review] > Bug 33020: Add test Note that www tests will need to export KOHA_INTRANET_URL because of temporary ktd's issue (see #335). *** Bug 32490 has been marked as a duplicate of this bug. *** Created attachment 147116 [details] [review] Bug 33020: Add missing Koha::Logger->history This patch is fixing the problem and makes the test passes. However we lost some debug info we had before. Prior to 22678 we had ==> /var/log/koha/kohadev/plack-error.log <== [2023-02-22 10:03:14.20431] [4790] [trace] [Ez_NLp-F8prN] GET "/api/v1/acquisition/vendors" [2023-02-22 10:03:14.21279] [4790] [trace] [Ez_NLp-F8prN] Template "not_found.development.html.ep" not found [2023-02-22 10:03:14.21307] [4790] [trace] [Ez_NLp-F8prN] Template "not_found.html.ep" not found [2023-02-22 10:03:14.21327] [4790] [trace] [Ez_NLp-F8prN] Rendering template "mojo/debug.html.ep" [2023-02-22 10:03:14.39498] [4790] [trace] [Ez_NLp-F8prN] 404 Not Found (0.190631s, 5.246/s) ==> /var/log/koha/kohadev/plack.log <== 172.19.0.1 - - [22/Feb/2023:10:03:14 +0000] "GET /api/v1/app.pl/api/v1/acquisition/vendors HTTP/1.1" 404 - "-" "curl/7.68.0" Now we only see the 404 from plack.log Not ready for testing, see last comment. (In reply to Jonathan Druart from comment #7) > Not ready for testing, see last comment. [2023/02/22 12:53:13] [WARN] ERROR: Unsupported method history at /usr/share/koha/Koha/Logger.pm line 135. [2023/02/22 13:06:22] [TRACE] GET "/api/v1/doesnt" [2023/02/22 13:06:22] [TRACE] Template "not_found.development.html.ep" not found [2023/02/22 13:06:22] [TRACE] Template "not_found.html.ep" not found [2023/02/22 13:06:22] [TRACE] Rendering template "mojo/debug.html.ep" [2023/02/22 13:06:22] [TRACE] 404 Not Found (0.273781s, 3.653/s) Check your log4perl.conf log4perl.logger.plack-api = TRACE, PLACKAPI log4perl.appender.PLACKAPI=Log::Log4perl::Appender::File log4perl.appender.PLACKAPI.filename=/var/log/koha/myclone/plack-api-error.log log4perl.appender.PLACKAPI.mode=append log4perl.appender.PLACKAPI.layout=PatternLayout log4perl.appender.PLACKAPI.layout.ConversionPattern=[%d] [%p] %m%n log4perl.appender.PLACKAPI.utf8=1 When I set trace, I get trace :) Created attachment 147138 [details] [review] Bug 33020: Add test Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 147139 [details] [review] Bug 33020: Add missing Koha::Logger->history Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 147402 [details] [review] Bug 33020: Add test Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 147403 [details] [review] Bug 33020: Add missing Koha::Logger->history Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 147404 [details] [review] Bug 33020: (QA follow-up) POD and chmod Pushed to master for 23.05. Nice work everyone, thanks! (In reply to Marcel de Rooy from comment #9) > Check your log4perl.conf > > log4perl.logger.plack-api = TRACE, PLACKAPI > log4perl.appender.PLACKAPI=Log::Log4perl::Appender::File > log4perl.appender.PLACKAPI.filename=/var/log/koha/myclone/plack-api-error.log > log4perl.appender.PLACKAPI.mode=append > log4perl.appender.PLACKAPI.layout=PatternLayout > log4perl.appender.PLACKAPI.layout.ConversionPattern=[%d] [%p] %m%n > log4perl.appender.PLACKAPI.utf8=1 > > When I set trace, I get trace :) This then was the behaviour before this patch. Shouldn't we have it back for dev installs? (In reply to Jonathan Druart from comment #16) > (In reply to Marcel de Rooy from comment #9) > > Check your log4perl.conf > > > > log4perl.logger.plack-api = TRACE, PLACKAPI > > log4perl.appender.PLACKAPI=Log::Log4perl::Appender::File > > log4perl.appender.PLACKAPI.filename=/var/log/koha/myclone/plack-api-error.log > > log4perl.appender.PLACKAPI.mode=append > > log4perl.appender.PLACKAPI.layout=PatternLayout > > log4perl.appender.PLACKAPI.layout.ConversionPattern=[%d] [%p] %m%n > > log4perl.appender.PLACKAPI.utf8=1 > > > > When I set trace, I get trace :) > > This then was the behaviour before this patch. Shouldn't we have it back for > dev installs? We relied on Mojo for that. I do not really see the need of building it into Koha personally, since you can adjust it very easily (just one sed line in your Dockerfile or so). But you are welcome to open a new report. This does not only pertain to this PLACKAPI setting but probably for others too? I am just saying there is a behaviour change caused by this patch, we lost debug info. That was not expected, and that's actually why I didn't want to push the patches. (In reply to Jonathan Druart from comment #18) > I am just saying there is a behaviour change caused by this patch, we lost > debug info. That was not expected, and that's actually why I didn't want to > push the patches. I dont understand. Compare comment8 and comment9. What is missing? (In reply to Marcel de Rooy from comment #19) > (In reply to Jonathan Druart from comment #18) > > I am just saying there is a behaviour change caused by this patch, we lost > > debug info. That was not expected, and that's actually why I didn't want to > > push the patches. > > I dont understand. Compare comment8 and comment9. What is missing? comment6 and comment8 In master there is no TRACE, only the "GET 404" line. (In reply to Jonathan Druart from comment #21) > In master there is no TRACE, only the "GET 404" line. I give up :) We are not on the same page here. (In reply to Marcel de Rooy from comment #22) > (In reply to Jonathan Druart from comment #21) > > In master there is no TRACE, only the "GET 404" line. > > I give up :) We are not on the same page here. I've made KTD set TRACE by default for the API. Hope it helps. I have reverted my patch for KTD for now. I'm getting this: ERROR: you didn't tell me how to implement your appender 'TRACE' at /usr/share/perl5/Log/Log4perl/Config.pm line 367. Can you clarify? Look here for reference on what I tried (now commented out): https://gitlab.com/koha-community/koha-testing-docker/-/commit/85e3e54f89a7b3341e948311a904ac875776d9ba (In reply to Tomás Cohen Arazi from comment #24) > I have reverted my patch for KTD for now. I'm getting this: > > ERROR: you didn't tell me how to implement your appender 'TRACE' at > /usr/share/perl5/Log/Log4perl/Config.pm line 367. > > Can you clarify? > > Look here for reference on what I tried (now commented out): > > https://gitlab.com/koha-community/koha-testing-docker/-/commit/ > 85e3e54f89a7b3341e948311a904ac875776d9ba Does not look good. You should specifiy one level. This level and higher you will get. So TRACE includes WARN. Not the other way around. Nice work everyone! Pushed to stable for 22.11.x Missing dependencies for 22.05.x, no backport. (In reply to Marcel de Rooy from comment #25) > (In reply to Tomás Cohen Arazi from comment #24) > > I have reverted my patch for KTD for now. I'm getting this: > > > > ERROR: you didn't tell me how to implement your appender 'TRACE' at > > /usr/share/perl5/Log/Log4perl/Config.pm line 367. > > > > Can you clarify? > > > > Look here for reference on what I tried (now commented out): > > > > https://gitlab.com/koha-community/koha-testing-docker/-/commit/ > > 85e3e54f89a7b3341e948311a904ac875776d9ba > > Does not look good. You should specifiy one level. This level and higher you > will get. So TRACE includes WARN. Not the other way around. Thanks, I added TRACE now, hope it works for everyone: https://gitlab.com/koha-community/koha-testing-docker/-/commit/28af8d92b92b1e9ed0daae144ca5c6e9154a05f2#f8e5d7813c78b88a7f843779149fcc428aded6bf_147_144 |