[12.0][FIX] - report_py3o: run libreoffice in an isolated user installation #365
Conversation
sbejaoui
changed the title
report_py3o/models/py3o_report.py
Outdated
| self.ir_actions_report_id.py3o_filetype, | ||
| result_path, | ||
| ], | ||
| tmp_user_installation, |
There was a problem hiding this comment.
Why do we need to return tmp_user_installation to the caller? Can't we clean it here?
There was a problem hiding this comment.
no, we need this to exist for the actual call to libreoffice to succeed. But I'm also concerned about breaking the API here, I'd prefer to see the the temporary directory passed in an optional keyword argument. If passed, we add the parameter to the command, and the caller is responsible for cleaning up.
There was a problem hiding this comment.
right, I skimmed on it too quickly. So yes, better doing a with TemporaryDirectory() in the caller and pass the temp dir to _convert_single_report.
There was a problem hiding this comment.
I pushed a new version to not break the API
|
I'm a little bit surprised by the repeated mention of the fusion server, as this is exactly the code path where we don't use the fusion server. Myself I never encountered the issue described, but then we also use this only for a handful of reports that are very unlikely to be run anywhere close to parallel. That said, the bug description makes sense to me, I immediately believe this can be a problem. So as soon as this doesn't break the API, I'd be fine with the change. |
sbejaoui
force-pushed
the
12.0-fix_py3o_print_simultaneously
branch
from
77a74ec
to
a40314c
Compare
Thank you for your review, I misunderstood the usage of the fusion server as I am not familiar with the module. I updated the PR description. |
sbejaoui
force-pushed
the
12.0-fix_py3o_print_simultaneously
branch
from
a40314c
to
6b137a0
Compare
There was a problem hiding this comment.
Thank you for this fix @sbejaoui. All this makes sens according to https://ask.libreoffice.org/en/question/14841/how-can-i-use-soffice-from-the-command-line-when-quickstarter-is-running/
…lation Bug when more than one conversion print is launched within the same libreoffice instance. The standard behavior of libreoffice when a user open it while another instance is running is to show a new window and throw an error if a new instance is forced within the same user installation [see](https://bugs.documentfoundation.org/show_bug.cgi?id=37531). This implies a bug in report_py3o module when we call libreoffice at the same time for different documents. To reproduce this bug: **Case 1:** 1. Simultaneously print two documents. **Case 2:** 1. Run print jobs using job_queue module 2. Manually print another document **Case 3:** 2. Open libreoffice 3. Print a py3o report This PR creates a temporary user installation for each libreoffice conversion to bypass this limitation.
sbejaoui
force-pushed
the
12.0-fix_py3o_print_simultaneously
branch
from
6b137a0
to
e55f91a
Compare
|
/ocabot merge patch |
|
This PR looks fantastic, let's merge it! |
|
It looks like something changed on |
|
Congratulations, your PR was merged at dccd888. Thanks a lot for contributing to OCA. ❤️ |
Bug when more than one conversion print is launched within the same libreoffice instance.
The standard behavior of libreoffice when a user open it while another instance is
running is to show a new window and throw an error if a new instance is forced within
the same user installation see.
This implies a bug in report_py3o module when we call libreoffice at the same time for
different documents.
To reproduce this bug:
Case 1:
Case 2:
Case 3:
2. Open libreoffice
3. Print a py3o report
This PR creates a temporary user installation for each libreoffice conversion to bypass this limitation.
cc/ @sbidoul