Skip to content

Conversation

@mensfeld
Copy link
Member

No description provided.

mensfeld and others added 30 commits September 22, 2023 13:59
* kraft

* align with kraft
* implement purge for producer

* add extra spec
* producer transactions

* stability

* fix

* cleanup and specs
* use WeakMap reference and object reference

* store opaque in inner

* fix spec

* remove Ruby 2.6 support

* stabilize spec

* producer GC operations specs

* admin and consumer gc specs

* fix specs

* give time to update (test)

* test

* give time to update (test)

* stabilize specs

* stabilize specs

* stabilize specs

* stabilize specs

* stabilize specs

* bump version for future merger with rdkafka-ruby

* improve spec stability
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
mensfeld and others added 22 commits April 17, 2025 13:57
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
* bump librdkafka to 2.10.0

* use correct checksum

* remarks

* alignments
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
* align defaults with deprecs on librdkafka 2.10

* changelog fixes

* remarks

* fix last

* remarks

* fix desc

* mitigate logger

* use default cluster value
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
)

* specs and stuf

* normalize name

* remarks

* more specs
@mensfeld mensfeld requested a review from Copilot May 14, 2025 18:28
@mensfeld mensfeld self-assigned this May 14, 2025
@mensfeld mensfeld added bug producer Producer related issues labels May 14, 2025
@mensfeld
Copy link
Member Author

arg not this repo, this will receive the same as a cherry pick

@mensfeld mensfeld closed this May 14, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a post-close hook for native Kafka resources, standardizes error handling via RdkafkaError.validate!, adds transactional APIs to the producer, and enhances metadata support in consumer partition lists.

  • Add yield in NativeKafka.close to run cleanup code before destruction
  • Refactor manual response checks to use RdkafkaError.validate!/build
  • Implement transactional methods on Producer and support offset metadata in consumer code

Reviewed Changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lib/rdkafka/producer/delivery_handle.rb Fix error delivery report creation logic
lib/rdkafka/producer.rb Add init/begin/commit/abort transaction methods and refactor flush/purge/produce
lib/rdkafka/native_kafka.rb Yield to an optional block before destroying the native client
lib/rdkafka/metadata.rb Replace manual error raise with RdkafkaError.validate!
lib/rdkafka/error.rb Introduce build, build_from_c, validate!, and details attr
lib/rdkafka/consumer/topic_partition_list.rb Support metadata in topic-partition lists
lib/rdkafka/consumer/partition.rb Add metadata attribute and include it in to_s
lib/rdkafka/consumer.rb Replace explicit response checks with validate!
lib/rdkafka/bindings.rb Update FFI signatures (e.g., topic_partition_list_add) and add transaction bindings
lib/rdkafka/admin/incremental_alter_configs_report.rb Simplify error handling via validate!
lib/rdkafka/admin/describe_configs_report.rb Simplify error handling via validate!
lib/rdkafka/admin/delete_topic_handle.rb Use validate! instead of manual RdkafkaError.new
lib/rdkafka/admin/create_topic_handle.rb Use validate! instead of manual RdkafkaError.new
lib/rdkafka/admin/create_partitions_handle.rb Use validate! instead of manual RdkafkaError.new
lib/rdkafka/admin/config_binding_result.rb Change synonyms mapping to append into @synonyms
lib/rdkafka/admin.rb Remove an extra blank line
lib/rdkafka/abstract_handle.rb Switch raise_error to use validate!
karafka-rdkafka.gemspec Rename gem to karafka-rdkafka and update URIs
README.md Update version table to include unreleased and recent versions
CHANGELOG.md Rebase changelog entries under new version headings
Comments suppressed due to low confidence (1)

lib/rdkafka/producer/delivery_handle.rb:28

  • In the zero-response branch, DeliveryReport.new is called without passing the topic argument, causing parameters to shift (label becomes error). You should invoke it as DeliveryReport.new(partition, offset, topic, nil, label).
if self[:response] == 0


(1..pointer_to_size_t.read_int).map do |ar|
self.class.new synonyms_ptr[ar - 1]
@synonyms << self.class.new(synonyms_ptr[ar - 1])
Copy link

Copilot AI May 14, 2025

Choose a reason for hiding this comment

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

You're appending to @synonyms without initializing it; this will raise a NoMethodError. Initialize @synonyms = [] before the loop.

Copilot uses AI. Check for mistakes.
Comment on lines 68 to 71
# @param partitions_with_offsets [Hash<Integer, Integer>] The topic's partitions and offsets
# @param partitions_with_offsets [Array<Consumer::Partition>] The topic's partitions with offsets
# and metadata (if any)
#
Copy link

Copilot AI May 14, 2025

Choose a reason for hiding this comment

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

[nitpick] The @param tag for partitions_with_offsets is duplicated with different types (Hash vs Array). Consider consolidating into one or clarifying both overloads.

Suggested change
# @param partitions_with_offsets [Hash<Integer, Integer>] The topic's partitions and offsets
# @param partitions_with_offsets [Array<Consumer::Partition>] The topic's partitions with offsets
# and metadata (if any)
#
# @param partitions_with_offsets [Hash<Integer, Integer>, Array<Consumer::Partition>] The topic's partitions and offsets.
# If a Hash is provided, the keys represent partition numbers, and the values represent offsets.
# If an Array is provided, it should contain `Consumer::Partition` objects, which may include offsets
# and metadata (if any).

Copilot uses AI. Check for mistakes.
@mensfeld mensfeld deleted the post-close-yield-fix branch May 14, 2025 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug producer Producer related issues

Development

Successfully merging this pull request may close these issues.

5 participants