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

support nil logger #39

Merged
merged 6 commits into from
Sep 28, 2024
Merged

support nil logger #39

merged 6 commits into from
Sep 28, 2024

Conversation

navidemad
Copy link
Contributor

@navidemad navidemad commented Sep 25, 2024

This feature has been implemented in solid_queue gem (rails/solid_queue#175)
I replicated the implementation for this gem.
FIxes rails/solid_cable/issues/38

@rx_adapter.send(:listener).send(:broadcast_messages)
end

assert true
Copy link
Contributor Author

@navidemad navidemad Sep 25, 2024

Choose a reason for hiding this comment

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

No logger, nothing to check, just ensure it runs without errors

private
def cable_config
{ adapter: "solid_cable", message_retention: "1.second",
polling_interval: "0.01.seconds" }
polling_interval: "0.01.seconds", silence_polling: true }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's the default value, but needed to add it there for my mocking method with_active_record_logger

if ::SolidCable.silence_polling?
ActiveRecord::Base.logger.silence { yield }
if ::SolidCable.silence_polling? && ActiveRecord::Base.logger
if ActiveRecord::Base.logger.respond_to?(:silence)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was getting an error, as silence is not present in rails application (7.2.1):

'with_polling_volume': undefined method `silence' for an instance of Logger (NoMethodError)
ActiveRecord::Base.logger.silence { yield }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If this PR is merged i will do a PR to fix silence also for those who want to use solid_queue in earlier version that Rails 8

@navidemad
Copy link
Contributor Author

@npezza93
To better reflect the intent of asserting that no exceptions are raised, I changed the test name to indicate that we are specifically testing for the absence of errors when there's no Active Record logger. Also, I ensure that the message is successfully broadcast to the queue. This approach ensures that the test covers the intended behavior without diving into private methods and focuses on preventing exceptions.

Copy link
Collaborator

@npezza93 npezza93 left a comment

Choose a reason for hiding this comment

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

Thanks

@npezza93 npezza93 merged commit bd349b3 into rails:main Sep 28, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

undefined method `silence' for nil (NoMethodError)
2 participants