Skip to content

[16.0] [MIG] queue_job and test_queue_job#460

Merged
OCA-git-bot merged 4 commits into
OCA:16.0from
acsone:16.0-mig-queue_job-bai
Oct 11, 2022
Merged

[16.0] [MIG] queue_job and test_queue_job#460
OCA-git-bot merged 4 commits into
OCA:16.0from
acsone:16.0-mig-queue_job-bai

Conversation

@baimont

@baimont baimont commented Oct 4, 2022

Copy link
Copy Markdown
Contributor

@baimont baimont mentioned this pull request Oct 4, 2022
Merged
hugosantosred
hugosantosred reviewed Oct 5, 2022
View reviewed changes

@hugosantosred hugosantosred left a comment

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 for the migration. Apart from that comment looks good to me

Comment thread queue_job/tests/test_model_job_channel.py Outdated
@baimont baimont force-pushed the 16.0-mig-queue_job-bai branch from a786a5d to ed9f633 Compare October 5, 2022 09:07
@baimont baimont requested a review from hugosantosred October 5, 2022 09:08
@guewen

guewen commented Oct 11, 2022

Copy link
Copy Markdown
Member

Hi, thanks for your work!
Could you migrate test_queue_job as well?
If it is not migrated at the same time, we have no way to tell if queue_job behaves as expected as most of the tests are there.

@baimont baimont changed the title [16.0] [MIG] queue_job [16.0] [MIG] queue_job and test_queue_job Oct 11, 2022
@baimont

baimont commented Oct 11, 2022

Copy link
Copy Markdown
Contributor Author

Hi, thanks for your work! Could you migrate test_queue_job as well? If it is not migrated at the same time, we have no way to tell if queue_job behaves as expected as most of the tests are there.

Hi @guewen, done

guewen
guewen reviewed Oct 11, 2022
View reviewed changes
Comment thread requirements.txt Outdated
@@ -1 +1,3 @@
# generated from manifests external_dependencies
mock

@guewen guewen Oct 11, 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.

@baimont can't we replace the import with from unittest import mock?

@baimont baimont Oct 11, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@guewen yes this is better. I adapted the commit

@baimont baimont force-pushed the 16.0-mig-queue_job-bai branch from eeba81e to 6caca1a Compare October 11, 2022 14:10
guewen
guewen approved these changes Oct 11, 2022
View reviewed changes

@guewen guewen left a comment

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.

Great! Many thanks!

@guewen

guewen commented Oct 11, 2022

Copy link
Copy Markdown
Member

/ocabot merge nobump

@OCA-git-bot

OCA-git-bot commented Oct 11, 2022

Copy link
Copy Markdown
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-460-by-guewen-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit cafc5c9 into OCA:16.0 Oct 11, 2022
@OCA-git-bot

OCA-git-bot commented Oct 11, 2022

Copy link
Copy Markdown
Contributor

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

@guewen

guewen commented Oct 11, 2022

Copy link
Copy Markdown
Member

@baimont did you check if jobs were executed? I'm testing locally (I have been enthusiastic about merging 😅), but jobs aren't executed. Now, I may have botched my install

@baimont

baimont commented Oct 11, 2022

Copy link
Copy Markdown
Contributor Author

@baimont did you check if jobs were executed? I'm testing locally (I have been enthusiastic about merging 😅), but jobs aren't executed. Now, I may have botched my install

Ahahaha I always execute them myself with pytest-odoo before pushing any change, so yes

@guewen

guewen commented Oct 11, 2022

Copy link
Copy Markdown
Member

That's nice, but sorry I speak about jobs ;)

@baimont

baimont commented Oct 11, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

That's nice, but sorry I speak about jobs ;)

Sorry I misread.
I verified through the web client with https://github.com/acsone/queue/blob/6caca1aa057bc38706af3f92c33ee51c5c6c2c7f/queue_job/controllers/main.py#L110 but not sure that's enough.

@okuryan

okuryan commented Oct 14, 2022

Copy link
Copy Markdown

@pedrobaeza could you, please, publish this repo on odoo app store?

@pedrobaeza

pedrobaeza commented Oct 14, 2022

Copy link
Copy Markdown
Member

That should be @dreispt or @simahawk if I don't remember bad.

@okuryan

okuryan commented Oct 15, 2022

Copy link
Copy Markdown

@dreispt , @simahawk could you, please, publish queue_job to odoo app store?

@sbidoul sbidoul deleted the 16.0-mig-queue_job-bai branch October 15, 2022 13:14
@sbidoul

sbidoul commented Oct 15, 2022

Copy link
Copy Markdown
Member

@baimont could you also look for commits in 14.0 that where not in the 15.0 branch? The oca-port tool may help for that.

For instance #443 is missing, it seems.

@baimont

baimont commented Oct 15, 2022

Copy link
Copy Markdown
Contributor Author

@baimont could you also look for commits in 14.0 that where not in the 15.0 branch? The oca-port tool may help for that.

For instance #443 is missing, it seems.

this pr is merged so i guess this has to be done in a new pr

@dreispt

dreispt commented Oct 17, 2022

Copy link
Copy Markdown
Member

@okuryan @pedrobaeza This repo is already published in Odoo Apps.
See for example: https://apps.odoo.com/apps/modules/15.0/queue_job/

@pedrobaeza

pedrobaeza commented Oct 17, 2022

Copy link
Copy Markdown
Member

@dreispt you have to publish each branch of each repo...

@okuryan

okuryan commented Oct 17, 2022

Copy link
Copy Markdown

@dreispt please, let me know when it is published for 16.0 ...

@okuryan

okuryan commented Oct 19, 2022

Copy link
Copy Markdown

@pedrobaeza are their any ways to reach person responsible for publishing? Or maybe how I can help to speed up publishing process?

@pedrobaeza

pedrobaeza commented Oct 19, 2022

Copy link
Copy Markdown
Member

Sorry, I can't say, as I'm not on OCA board, and don't know other OCA apps store managers.

@dreispt

dreispt commented Oct 19, 2022

Copy link
Copy Markdown
Member

I tried again and we can now publish 16.0 branches,
It was done for the Queue repo, and this module is now available:
https://apps.odoo.com/apps/modules/16.0/queue_job/

@okuryan

okuryan commented Oct 19, 2022

Copy link
Copy Markdown

@dreispt yes, I had to write to Odoo support for this and push them through our account manager. They had bug on Odoo app store not allowing to publish Odoo 16

@baimont

baimont commented Oct 19, 2022

Copy link
Copy Markdown
Contributor Author

@baimont could you also look for commits in 14.0 that where not in the 15.0 branch? The oca-port tool may help for that.

For instance #443 is missing, it seems.

I did a fw port of #443 here : #470
And then I saw this : #453

@sbidoul

sbidoul commented Oct 20, 2022

Copy link
Copy Markdown
Member

@baimont I think I meant #433, actually. Sorry for the confusion.

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

Reviewers

@guewen guewen guewen approved these changes

+1 more reviewer

@hugosantosred hugosantosred hugosantosred approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

approved merged 🎉

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@baimont @guewen @OCA-git-bot @okuryan @pedrobaeza @sbidoul @dreispt @hugosantosred