Skip to content

PYTHON-4701 - Topology logging should use suppress_event #1826

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
Sep 3, 2024

Conversation

NoahStapp
Copy link
Contributor

No description provided.

@@ -497,7 +497,7 @@ async def _process_change(
(td_old, self._description, self._topology_id),
)
)
if _SDAM_LOGGER.isEnabledFor(logging.DEBUG):
if _SDAM_LOGGER.isEnabledFor(logging.DEBUG) and sd_old != server_description:
Copy link
Member

Choose a reason for hiding this comment

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

Should this be and not suppress_event?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

suppress_event checks for event publishing flags, which we don't use for logging.

Copy link
Member

Choose a reason for hiding this comment

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

Could that be changed?

suppress_event = sd_old == server_description

@ShaneHarvey
Copy link
Member

Also could you add some more info to the failing assert message?

@NoahStapp
Copy link
Contributor Author

Also could you add some more info to the failing assert message?

To which message?

@ShaneHarvey
Copy link
Member

The one in this ticket:

TestUnifiedLoggingStandalone.test_Topology_lifecycle _____________

self = <test.test_discovery_and_monitoring.TestUnifiedLoggingStandalone testMethod=test_Topology_lifecycle>

    def test_case(self):
>       self.run_scenario(spec)

test/unified_format.py:2074: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
test/unified_format.py:2018: in run_scenario
    self._run_scenario(spec, uri)
test/unified_format.py:2048: in _run_scenario
    self.check_log_messages(spec["operations"], expect_log_messages)
test/unified_format.py:1957: in check_log_messages
    self.assertEqual(len(client["messages"]), len(actual_logs))
E   AssertionError: 7 != 8

@ShaneHarvey
Copy link
Member

ShaneHarvey commented Sep 3, 2024

Could you update the PR title and Jira ticket to describe the bug fix instead of the test failure name?

@NoahStapp NoahStapp changed the title PYTHON-4701 - Fix TestUnifiedLoggingStandalone.test_Topology_lifecycle PYTHON-4701 - Topology logging should use suppress_event Sep 3, 2024
@NoahStapp NoahStapp merged commit 5a70039 into mongodb:master Sep 3, 2024
31 of 32 checks passed
@NoahStapp NoahStapp deleted the PYTHON-4701 branch September 3, 2024 20:57
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.

2 participants