Skip to content

RUBY-2458 Link heartbeat succeeded/failed to started events #2147

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

Merged
merged 2 commits into from
Dec 3, 2020

Conversation

p-mongo
Copy link
Contributor

@p-mongo p-mongo commented Nov 27, 2020

No description provided.

@p-mongo p-mongo changed the title RUBY-2458 2458 RUBY-2458 Link heartbeat succeeded/failed to started events Nov 27, 2020
@p-mongo
Copy link
Contributor Author

p-mongo commented Nov 27, 2020

@p-mongo p-mongo requested a review from durran December 1, 2020 01:35
Copy link
Member

@durran durran left a comment

Choose a reason for hiding this comment

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

Just a few nits - doesn't look like the Evergreen failures are directly related to this PR so LGTM after those 2 comments.

@@ -328,14 +328,20 @@ def publish_heartbeat(server, awaited: false)
rescue => exc
if monitoring?
event = Event::ServerHeartbeatFailed.new(
server.address, Time.now-start_time, exc, awaited: awaited)
server.address, Time.now-start_time, exc,
Copy link
Member

Choose a reason for hiding this comment

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

Bit of a nit but I would expect all parameters do be on new lines or not - not a combination of them.

failed(SERVER_HEARTBEAT, event)
end
raise
else
if monitoring?
event = Event::ServerHeartbeatSucceeded.new(
server.address, Time.now-start_time, awaited: awaited)
server.address, Time.now-start_time,
Copy link
Member

Choose a reason for hiding this comment

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

Same as above with parameters.

@p-mongo
Copy link
Contributor Author

p-mongo commented Dec 2, 2020

All of the positional parameters were on the same line but I made each of them take up their own line as you requested.

Copy link
Member

@durran durran left a comment

Choose a reason for hiding this comment

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

LGTM

@p-mongo p-mongo merged commit c344fae into mongodb:master Dec 3, 2020
p-mongo pushed a commit to p-mongo/mongo-ruby-driver that referenced this pull request Dec 3, 2020
* master:
  RUBY-2459 Include connection purpose in platform metadata (mongodb#2148)
  RUBY-2458 Link heartbeat succeeded/failed to started events (mongodb#2147)
  RUBY-2446 Fix intermittent SDAM test failures (mongodb#2145)
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.

3 participants