Skip to content

[16.0] queue_job: unify no delay option - #521

Merged
OCA-git-bot merged 1 commit into
OCA:16.0from
camptocamp:16-qjob-fix-disable-key
Apr 2, 2023
Merged

[16.0] queue_job: unify no delay option#521
OCA-git-bot merged 1 commit into
OCA:16.0from
camptocamp:16-qjob-fix-disable-key

Conversation

@simahawk

Copy link
Copy Markdown
Contributor

Current situation:

  • multiple keys for no good reason
  • half baked: not all of them used everywhere
  • no centralization
  • poor naming

With this change we'll have:

  • 1 and only one key to disable via ctx: queue_job__no_delay
  • 1 and only one key to disable via os env: QUEUE_JOB__NO_DELAY
  • backward compatibility with deprecation for old keys

@simahawksimahawk added this to the 16.0 milestone Feb 28, 2023
@OCA-git-bot

Copy link
Copy Markdown
Contributor

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

@simahawk
simahawkforce-pushed the 16-qjob-fix-disable-key branch from bef1eb4 to 5d03eaaCompareFebruary 28, 2023 14:36
Current situation:
* multiple keys for no good reason
* half baked: not all of them used everywhere
* no centralization
* poor naming
With this change we'll have:
* 1 and only one key to disable via ctx: ``queue_job__no_delay``
* 1 and only one key to disable via os env: ``QUEUE_JOB__NO_DELAY``
* backward compatibility with deprecation for old keys
@simahawk
simahawkforce-pushed the 16-qjob-fix-disable-key branch from 5d03eaa to dff747cCompareFebruary 28, 2023 14:38

@grindtildeathgrindtildeath left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good overall. Is there any reason to use a double underscore in the name of ctx key and env var?
IMO it would be better to have a single underscore as in QUEUE_JOB_NO_DELAY to avoid potential troubles because it's not something that's easy to detect if you accidentally write only one.

@simahawk

Copy link
Copy Markdown
ContributorAuthor

Actually the env var has 2 underscores too. TBH this is just an approach I'm taking to identify keys by module especially if the are meant to be used in many many modules. So the __ is just to separate the name explicitly.
I can change that of course.

@guewen

Copy link
Copy Markdown
Member

/ocabot merge minor

@OCA-git-bot

Copy link
Copy Markdown
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-521-by-guewen-bump-minor, awaiting test results.

@OCA-git-bot

Copy link
Copy Markdown
Contributor

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

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@simahawk@OCA-git-bot@guewen@theangryangel@grindtildeath