Skip to content

Conversation

@vincent-cowboy
Copy link

@vincent-cowboy vincent-cowboy commented Jan 17, 2023

Inspired by #492

Help needed regarding RuntimeError("object unbound") when using xls format:

Traceback (most recent call last):
 (...)
  File "(...)/odoo15/oca/queue/base_export_async/models/delay_export.py", line 82, in export
    content = self._get_file_content(params)
  File "(...)/odoo15/oca/queue/base_export_async/models/delay_export.py", line 66, in _get_file_content
    return xlsx.from_data(columns_headers, import_data)
  File "(...)/odoo15/odoo/addons/web/controllers/main.py", line 1913, in from_data
    with ExportXlsxWriter(fields, len(rows)) as xlsx_writer:
  File "(...)/odoo15/odoo/addons/web/controllers/main.py", line 709, in __init__
    decimal_places = [res['decimal_places'] for res in request.env['res.currency'].search_read([], ['decimal_places'])]
  File "(...)/.pyenv/versions/odoo15/lib/python3.8/site-packages/werkzeug/local.py", line 348, in __getattr__
    return getattr(self._get_current_object(), name)
  File "(...)/.pyenv/versions/odoo15/lib/python3.8/site-packages/werkzeug/local.py", line 307, in _get_current_object
    return self.__local()
  File "(...)/.pyenv/versions/odoo15/lib/python3.8/site-packages/werkzeug/local.py", line 137, in _lookup
    raise RuntimeError("object unbound")
RuntimeError: object unbound

Guewen Baconnier and others added 13 commits January 17, 2023 12:34
The UI still send it to the current user, but it will allow
reusing the module for other purposes: plan a regular export
to users.

Setting the recipient_ids instead of the email_to in mail.mail means
that we can still send the email, it will stay in the failed emails,
allowing to correct the user.
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: queue-12.0/queue-12.0-base_export_async
Translate-URL: https://translation.odoo-community.org/projects/queue-12-0/queue-12-0-base_export_async/
Currently translated at 100.0% (18 of 18 strings)

Translation: queue-12.0/queue-12.0-base_export_async
Translate-URL: https://translation.odoo-community.org/projects/queue-12-0/queue-12-0-base_export_async/fr/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: queue-12.0/queue-12.0-base_export_async
Translate-URL: https://translation.odoo-community.org/projects/queue-12-0/queue-12-0-base_export_async/
Currently translated at 100.0% (18 of 18 strings)

Translation: queue-12.0/queue-12.0-base_export_async
Translate-URL: https://translation.odoo-community.org/projects/queue-12-0/queue-12-0-base_export_async/da/
Currently translated at 94.4% (17 of 18 strings)

Translation: queue-12.0/queue-12.0-base_export_async
Translate-URL: https://translation.odoo-community.org/projects/queue-12-0/queue-12-0-base_export_async/pt/
@amh-mw
Copy link
Member

amh-mw commented Jan 21, 2023

I investigated "object unbound" errors a couple weeks ago in the server-auth repository: OCA/server-auth#456 (review). We had to push a mock request onto the stack for testing, i.e.

from odoo.http import _request_stack

// in setUp() 
_request_stack.push(mock.Mock(
    env=self.env,
))
self.addCleanup(_request_stack.pop)

@vincent-cowboy vincent-cowboy force-pushed the 15.0-mig-base_export_async branch from ed7c183 to 024a3cb Compare January 24, 2023 13:35
@vincent-cowboy
Copy link
Author

@amh-mw Thank you so much for your help 🙏
As the RuntimeError("object unbound") is raised not only with the test suite but actually when using the module, I've added _request_stack.push(self) in the export method:

_request_stack.push(self)  # prevents RuntimeError("object unbound")
content = self._get_file_content(params)
_request_stack.pop()

To be honest, my knowledge on the LocalStack object is limited.
I'm unsure if it is the proper solution here. What do you think?

@amh-mw
Copy link
Member

amh-mw commented Jan 24, 2023

No, it doesn't seem quite right to me. When a queue_job worker delegates jobs, it requests a web worker to actually do the work, so I am surprised that there is no request on the stack. I can't spare a lot of time, but let me read through the code real quick and see if I get any ideas.

@vincent-cowboy
Copy link
Author

I've put the finger on the issue: I'm using queue_job_cron_jobrunner instead of queue_job.
RuntimeError("object unbound") is raised with the former, not with the latter.

@amh-mw
Copy link
Member

amh-mw commented Jan 24, 2023

Ah, that makes a lot of sense. I think the best course of action forward would be to move the _request_stack.push bandage into queue_job_cron_jobrunner, which would move it into another pull request entirely. That module is marked alpha / non-production anyway and the second issue would be a good design talking point.

@vincent-cowboy vincent-cowboy force-pushed the 15.0-mig-base_export_async branch 2 times, most recently from 6cbfdd4 to a82da95 Compare January 24, 2023 14:46
Copy link
Member

@amh-mw amh-mw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand the urge to refactor, but I'm not sure that I agree with behavior changes in a migration commit.

@vincent-cowboy vincent-cowboy force-pushed the 15.0-mig-base_export_async branch from a82da95 to 2eff778 Compare January 25, 2023 09:19
@vincent-cowboy
Copy link
Author

@amh-mw I've moved the email template refactoring into a separate commit, but let me know if it actually should be into a separate PR.

The method cron_delete(self) is reverted to its initial content.

@vincent-cowboy vincent-cowboy force-pushed the 15.0-mig-base_export_async branch 2 times, most recently from 145d12a to 0aff65c Compare January 25, 2023 09:32
Copy link
Member

@amh-mw amh-mw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. @OCA/tools-maintainers Ready for review and merge.

@amh-mw
Copy link
Member

amh-mw commented Jan 25, 2023

LGTM. @OCA/connector-maintainers Ready for review and merge. Apologies to mistakenly notified tools-maintainers.

@cuongnmtm
Copy link

Thanks @vincent-cowboy for this PR! I have copied the changes of the automatic test to bypass the object unbound issue to my PR #492 .

@cuongnmtm
Copy link

@vincent-cowboy After install base_export_async, there is a defect when click on the export all button

Screen Shot 2023-01-31 at 11 15 57 AM

I fixed it on my PR for 14.0 migration c6b4499.

@vincent-cowboy vincent-cowboy force-pushed the 15.0-mig-base_export_async branch from 0aff65c to de2283f Compare January 31, 2023 14:38
@vincent-cowboy
Copy link
Author

@cuongnmtm nice catch and thank you for the heads-up! the change is now included

@vincent-cowboy vincent-cowboy force-pushed the 15.0-mig-base_export_async branch from de2283f to 4a4ec05 Compare February 3, 2023 08:22
@vincent-cowboy
Copy link
Author

migrations folder removed as specified in Tasks to do in the migration

@github-actions
Copy link

github-actions bot commented Jun 4, 2023

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Jun 4, 2023
@kikopeiro
Copy link
Contributor

LGTM 👍 . @OCA/connector-maintainers Could you merge this PR?

@thomaspaulb
Copy link

/ocabot migration base_export_async
/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Sorry @thomaspaulb you are not allowed to merge.

To do so you must either have push permissions on the repository, or be a declared maintainer of all modified addons.

If you wish to adopt an addon and become it's maintainer, open a pull request to add your GitHub login to the maintainers key of its manifest.

@OCA-git-bot
Copy link
Contributor

Sorry @thomaspaulb you are not allowed to mark the addon tobe migrated.

To do so you must either have push permissions on the repository, or be a declared maintainer of all modified addons.

If you wish to adopt an addon and become it's maintainer, open a pull request to add your GitHub login to the maintainers key of its manifest.

@dreispt
Copy link
Member

dreispt commented Jun 30, 2023

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 15.0-ocabot-merge-pr-498-by-dreispt-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit e0c7f1e into OCA:15.0 Jun 30, 2023
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at b8e8a11. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved merged 🎉 stale PR/Issue without recent activity, it'll be soon closed automatically.

Projects

None yet

Development

Successfully merging this pull request may close these issues.