action #76912
closedOpenQA::Script::Client throws perl warning "Wide character in print", should not be there
Description
Observation¶
https://gitlab.suse.de/openqa/auto-review/-/jobs/280560#L141
shows
Wide character in print at /usr/share/openqa/script/../lib/OpenQA/Script/Client.pm line 54.
Acceptance criteria¶
- AC1: No perl warnings
Suggestions¶
The code line says
print Cpanel::JSON::XS->new->allow_nonref->pretty->encode($json);
shouldn't be impossible to handle unicode properly in 2020 :)
Updated by okurz over 3 years ago
- Target version changed from Ready to future
tinita stated that likely the problem would not appear when switching to openqa-cli instead. I don't mind doing that but actually I think we can unplan this again for now.
Updated by okurz over 3 years ago
- Status changed from Workable to Feedback
- Assignee set to okurz
- Target version changed from future to Ready
ok, I understand now that this perl warning is somehow ending up in the output we collect in the file "to_review" which hence ends up bigger than 0 bytes and is therefore failing the check in the gitlab CI pipeline. So we should plan to handle that now to prevent too many failed alerts.
https://blog.ostermiller.org/perl-wide-character-in-print/ suggests to use either a cpan module utf8::all which we have in "devel:languages:perl" but not in Factory nor in Leap so we can either submit that to both or use the other mentioned approach which is to call perl -CSDA. Or we specify use open ":std", ":encoding(UTF-8)"
https://github.com/os-autoinst/openQA/pull/3591
Also applied a temporary patch in https://gitlab.suse.de/openqa/auto-review/-/merge_requests/5 which can be removed again after the above PR is merged and deployed into the container images we use in the gitlab CI pipelines
Updated by okurz over 3 years ago
- Related to action #77944: Run "auto-review" more often but alarm less added
Updated by livdywan over 3 years ago
I'm unsure if this makes sense. A work-around is available. And the fix might break encoding with other uses of the client and we planned to move away from the old client, which should count especially for maintained code.
Of course you might be prepared to rollback if needed...
Updated by okurz over 3 years ago
cdywan wrote:
I'm unsure if this makes sense. A work-around is available. And the fix might break encoding with other uses of the client and we planned to move away from the old client, which should count especially for maintained code.
so far I don't see that plan but you could create a ticket regarding the different clients and we can plan that accordingly. I would not start removing the old client as long as we have not marked it as deprecated and informed users or replaced it using a compatibility wrapper.
Updated by okurz over 3 years ago
- Due date set to 2020-12-09
https://github.com/os-autoinst/openQA/pull/3591 was merged. The container will only have the new version after the next submission to Factory is accepted and the next Tumbleweed snapshot is published which can take some days. Setting due date as reminder when to check again.
Updated by okurz over 3 years ago
- Status changed from Feedback to Resolved
with https://gitlab.suse.de/openqa/auto-review/-/merge_requests/7 I could remove the downstream patch as the patch is available from upstream.