Skip to content

[FIX] queue_job: delayable representation failed #487

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.

Sign up for GitHub

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

Jump to bottom
Merged
OCA-git-bot merged 1 commit into from
Dec 7, 2022
Merged

[FIX] queue_job: delayable representation failed #487

OCA-git-bot merged 1 commit into from
Dec 7, 2022

Conversation

@etobella
Copy link
Member

@etobella etobella commented Dec 1, 2022

Represenation might fall in case of no job definition (as it happens on https://github.com/OCA/edi/blob/15.0/edi_oca/tests/test_record.py#L127), it tries to generate the log warning, but it fails

@simahawk

@OCA-git-bot
Copy link
Contributor

OCA-git-bot commented Dec 1, 2022

Hi @guewen,
some modules you are maintaining are being modified, check this out!

@etobella etobella mentioned this pull request Dec 1, 2022
Merged
@etobella
Copy link
Member Author

etobella commented Dec 1, 2022

The error raised is the following:

https://github.com/OCA/edi/actions/runs/3593875771/jobs/6051404638#step:8:127

simahawk
simahawk approved these changes Dec 2, 2022
View reviewed changes
Copy link
Contributor

@simahawk simahawk left a comment

Choose a reason for hiding this comment

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

LG, maybe worth to adapt the tests?

guewen
guewen reviewed Dec 2, 2022
View reviewed changes
queue_job/delay.py Outdated
return "Delayable({}.{}({}, {}))".format(
self.recordset, self._job_method.__name__, self._job_args, self._job_kwargs
str(self.recordset),
self._job_method and self._job_method.__name__ or "",
Copy link
Member

@guewen guewen Dec 2, 2022

Choose a reason for hiding this comment

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

Suggested change
self._job_method and self._job_method.__name__ or "",
self._job_method.__name__ if self._job_method else "",

... and ... or ... is a pattern I prefer to avoid

guewen
guewen approved these changes Dec 2, 2022
View reviewed changes
Copy link
Member

@guewen guewen left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks! Would you mind applying my "cosmetic" suggestion?

sbidoul
sbidoul reviewed Dec 2, 2022
View reviewed changes
@OCA-git-bot
Copy link
Contributor

OCA-git-bot commented Dec 6, 2022

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@etobella
Copy link
Member Author

etobella commented Dec 7, 2022

Thanks!!! All comments attended

@guewen
Copy link
Member

guewen commented Dec 7, 2022

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

OCA-git-bot commented Dec 7, 2022

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 15.0-ocabot-merge-pr-487-by-guewen-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit bee08ac into OCA:15.0 Dec 7, 2022
@OCA-git-bot
Copy link
Contributor

OCA-git-bot commented Dec 7, 2022

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

This was referenced Feb 28, 2023
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sbidoul sbidoul sbidoul left review comments

@simahawk simahawk simahawk approved these changes

@guewen guewen guewen approved these changes

Assignees

No one assigned

Labels

approved merged 🎉 ready to merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@etobella @OCA-git-bot @guewen @simahawk @sbidoul