Skip to content

PYTHON-3232 Improved change stream event visibility for C2C Replication #1062

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 7 commits into from
Sep 27, 2022

Conversation

juliusgeo
Copy link
Contributor

No description provided.

@@ -40,6 +40,7 @@ def __init__(
user_fields=None,
result_processor=None,
comment=None,
show_expanded_events=None,
Copy link
Member

Choose a reason for hiding this comment

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

Since you're implementing PYTHON-3232 in this PR can you rename the title and close PYTHON-3373 as a duplicate of PYTHON-3232?

@juliusgeo juliusgeo changed the title PYTHON-3373 Expose rich field information in the updateDescription PYTHON-3232 Improved change stream event visibility for C2C Replication Sep 26, 2022
@blink1073
Copy link
Member

Can you merge from master to pick up #1063?

Copy link
Member

@ShaneHarvey ShaneHarvey left a comment

Choose a reason for hiding this comment

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

Let's add a changelog entry for show_expanded_events too.

@@ -175,6 +177,10 @@ def _change_stream_options(self):

if self._start_at_operation_time is not None:
options["startAtOperationTime"] = self._start_at_operation_time

if self._show_expanded_events is True:
Copy link
Member

Choose a reason for hiding this comment

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

bools should be compared with if x: or if not x: not if x is True: or if x is False:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about truthy values?

>>> if 1:
...     print("help")
... 
help

Copy link
Member

Choose a reason for hiding this comment

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

Yeah that's the point. if x works with anything truthy while if x is True doesn't. It's more verbose and less flexible.

Copy link
Member

Choose a reason for hiding this comment

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

See

Don’t compare boolean values to True or False using ==:
# Correct:
if greeting:
# Wrong:
if greeting == True:
Worse:

# Wrong:
if greeting is True:

https://peps.python.org/pep-0008/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, ok. I was under the impression we would want less flexibility in this case.

Copy link
Member

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

LGTM!

@juliusgeo juliusgeo merged commit c874c96 into mongodb:master Sep 27, 2022
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