Skip to content
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.

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

Fix name extraction for module queue adapters #47995

Merged
merged 1 commit into from Apr 20, 2023

Conversation

Mangara
Copy link
Contributor

@Mangara Mangara commented Apr 19, 2023

Motivation / Background

Currently, setting a module as queue adapter makes queue_adapter_name return "module", instead of a name derived from the name of the module.

Detail

This Pull Request fixes the name extraction logic to also derive a name for module or class queue adapters.

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

Before this change, setting a module as queue adapter would result in
`queue_adapter_name` being `"module"`. This PR fixes the name extraction logic
to handle module or class queue adapters.
@rails-bot rails-bot bot added the activejob label Apr 19, 2023
@@ -43,7 +43,8 @@ def queue_adapter=(name_or_adapter)
assign_adapter(name_or_adapter.to_s, queue_adapter)
else
if queue_adapter?(name_or_adapter)
adapter_name = "#{name_or_adapter.class.name.demodulize.remove('Adapter').underscore}"
adapter_class = name_or_adapter.is_a?(Module) ? name_or_adapter : name_or_adapter.class
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a better way to do this?

Copy link
Member

Choose a reason for hiding this comment

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

Nah, but it's ok.

@byroot byroot merged commit 0f764b4 into rails:main Apr 20, 2023
9 checks passed
Mangara added a commit to Mangara/rails that referenced this pull request May 2, 2023
We recently made improvements to the name extraction for `MyJob.queue_adapter_name` (see rails#47995 and rails#48003). I had overlooked that we also extract the queue adapter name in a similar (but not identical) way for log messages, so this change aligns the two.
briu pushed a commit to briu/rails that referenced this pull request May 4, 2023
We recently made improvements to the name extraction for `MyJob.queue_adapter_name` (see rails#47995 and rails#48003). I had overlooked that we also extract the queue adapter name in a similar (but not identical) way for log messages, so this change aligns the two.
Sign up for free to 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.

None yet

2 participants