Skip to content

[FIX] queue_job: delayable representation failed#487

Merged
OCA-git-bot merged 1 commit into
OCA:15.0from
tegin:15.0-fix-delay
Dec 7, 2022
Merged

[FIX] queue_job: delayable representation failed#487
OCA-git-bot merged 1 commit into
OCA:15.0from
tegin:15.0-fix-delay

Conversation

@etobella

@etobella etobella commented Dec 1, 2022

Copy link
Copy Markdown
Member

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

OCA-git-bot commented Dec 1, 2022

Copy link
Copy Markdown
Contributor

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

@etobella etobella mentioned this pull request Dec 1, 2022
Merged
@etobella

etobella commented Dec 1, 2022

Copy link
Copy Markdown
Member Author

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

@simahawk simahawk left a comment

Copy link
Copy Markdown
Contributor

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
Comment thread 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 "",

@guewen guewen Dec 2, 2022

Copy link
Copy Markdown
Member

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

@guewen guewen left a comment •
edited
Loading

Copy link
Copy Markdown
Member

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
Comment thread queue_job/delay.py Outdated
@OCA-git-bot

OCA-git-bot commented Dec 6, 2022

Copy link
Copy Markdown
Contributor

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

etobella commented Dec 7, 2022

Copy link
Copy Markdown
Member Author

Thanks!!! All comments attended

@guewen

guewen commented Dec 7, 2022

Copy link
Copy Markdown
Member

/ocabot merge patch

@OCA-git-bot

OCA-git-bot commented Dec 7, 2022

Copy link
Copy Markdown
Contributor

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

OCA-git-bot commented Dec 7, 2022

Copy link
Copy Markdown
Contributor

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