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

refactor: Make capture overflow partitioning code a bit more straightforward #21145

Closed
wants to merge 1 commit into from

Conversation

tkaemming
Copy link
Contributor

Problem

These might be personal problems, but:

  1. I've been writing Python for nearly two decades and still have to look up the precedence rules for logical operator chaining like this
  2. I misread the candidate_partition_key/kafka_partition_key assignments earlier (in part because my brain glossed over the complex conditional expression)

Changes

  1. Made the operator precedence explicit
  2. Tried to make it clearer there are two separate potential assignments for kafka_partition_key by keeping them closer together makes it glaringly obvious for the next unsuspecting reader.

Does this work well for both Cloud and self-hosted?

Yes.

How did you test this code?

Hopefully already covered, maybe?

@tkaemming tkaemming changed the title ref: Make capture overflow partitioning code a bit more straightforward refactor: Make capture overflow partitioning code a bit more straightforward Mar 25, 2024
@tkaemming
Copy link
Contributor Author

Going to rework this more significantly to address concerns with #20945 (comment).

@tkaemming tkaemming closed this Mar 26, 2024
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.

1 participant