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

V2 rebased #220

Draft
wants to merge 18 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 0 additions & 17 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ Lint/UselessAssignment:
Metrics/AbcSize:
Exclude:
- 'lib/deimos/active_record_consume/message_consumption.rb'
- 'lib/deimos/config/phobos_config.rb'
- 'lib/deimos/instrumentation.rb'
- 'lib/deimos/kafka_source.rb'
- 'lib/deimos/kafka_topic_info.rb'
Expand All @@ -159,12 +158,6 @@ Metrics/AbcSize:
- 'lib/deimos/utils/schema_controller_mixin.rb'
Copy link
Collaborator

Choose a reason for hiding this comment

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

inline_consumer and schema_controller_mixin can be removed as the files are removed in the PR.

    - 'lib/deimos/utils/inline_consumer.rb'
    - 'lib/deimos/utils/schema_controller_mixin.rb'

- 'lib/generators/deimos/schema_class_generator.rb'

# Offense count: 1
# Configuration parameters: CountComments, Max, CountAsOne, ExcludedMethods.
Metrics/MethodLength:
Exclude:
- 'lib/deimos/config/phobos_config.rb'

# Offense count: 5
# Configuration parameters: CountComments, Max, CountAsOne.
Metrics/ModuleLength:
Expand All @@ -179,7 +172,6 @@ Metrics/ModuleLength:
# Configuration parameters: IgnoredMethods, Max.
Metrics/PerceivedComplexity:
Exclude:
- 'lib/deimos/config/phobos_config.rb'
- 'lib/deimos/consume/batch_consumption.rb'
- 'lib/deimos/kafka_source.rb'
- 'lib/deimos/schema_backends/avro_schema_coercer.rb'
Expand Down Expand Up @@ -253,7 +245,6 @@ Style/FrozenStringLiteralComment:
Style/GlobalStdStream:
Exclude:
- 'lib/deimos/config/configuration.rb'
- 'lib/deimos/config/phobos_config.rb'
- 'lib/deimos/metrics/mock.rb'
- 'lib/deimos/test_helpers.rb'
- 'lib/deimos/tracing/mock.rb'
Expand Down Expand Up @@ -329,14 +320,6 @@ Style/StringLiterals:
- 'spec/schemas/my_namespace/my_schema_with_complex_type.rb'
- 'spec/spec_helper.rb'

# Offense count: 1
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle, AllowSafeAssignment.
# SupportedStyles: require_parentheses, require_no_parentheses, require_parentheses_when_complex
Style/TernaryParentheses:
Exclude:
- 'lib/deimos/config/phobos_config.rb'

# Offense count: 21
# Cop supports --auto-correct.
Style/TrailingBodyOnModule:
Expand Down
1 change: 1 addition & 0 deletions .tool-versions
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ruby 3.2.2
3 changes: 1 addition & 2 deletions deimos-ruby.gemspec
Copy link
Collaborator

Choose a reason for hiding this comment

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

I remember you pointed out specific ruby version to be a requirement. Would it be useful to add it to gemspec as a requirement?
spec.required_ruby_version = '>= 3.0.0'

Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,8 @@ Gem::Specification.new do |spec|
spec.require_paths = ['lib']

spec.add_runtime_dependency('avro_turf', '>= 1.4', '< 2')
spec.add_runtime_dependency('karafka', '~> 2.0')
spec.add_runtime_dependency('fig_tree', '~> 0.0.2')
spec.add_runtime_dependency('phobos', '>= 1.9', '< 3.0')
spec.add_runtime_dependency('ruby-kafka', '< 2')
spec.add_runtime_dependency('sigurd', '>= 0.1.0', '< 1.0')

spec.add_development_dependency('activerecord-import')
Expand Down
11 changes: 1 addition & 10 deletions lib/deimos.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
# frozen_string_literal: true

require 'active_support'
require 'karafka'

require 'phobos'
require 'deimos/version'
require 'deimos/config/configuration'
require 'deimos/producer'
Expand All @@ -23,7 +23,6 @@
require 'deimos/schema_class/enum'
require 'deimos/schema_class/record'

require 'deimos/monkey_patches/phobos_cli'

require 'deimos/railtie' if defined?(Rails)
require 'deimos/utils/schema_controller_mixin' if defined?(ActionController)
Expand Down Expand Up @@ -117,13 +116,5 @@ def start_db_backend!(thread_count: 1)
end
end

at_exit do
begin
Deimos::Backends::KafkaAsync.shutdown_producer
Deimos::Backends::Kafka.shutdown_producer
rescue StandardError => e
Deimos.config.logger.error(
"Error closing producer on shutdown: #{e.message} #{e.backtrace.join("\n")}"
)
end
end
9 changes: 0 additions & 9 deletions lib/deimos/backends/kafka.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,6 @@ module Deimos
module Backends
# Default backend to produce to Kafka.
class Kafka < Base
include Phobos::Producer

# Shut down the producer if necessary.
# @return [void]
def self.shutdown_producer
producer.sync_producer_shutdown if producer.respond_to?(:sync_producer_shutdown)
producer.kafka_client&.close
end

# :nodoc:
def self.execute(producer_class:, messages:)
Deimos.instrument(
Expand Down
9 changes: 0 additions & 9 deletions lib/deimos/backends/kafka_async.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,6 @@ module Deimos
module Backends
# Backend which produces to Kafka via an async producer.
class KafkaAsync < Base
include Phobos::Producer

# Shut down the producer cleanly.
# @return [void]
def self.shutdown_producer
producer.async_producer_shutdown
producer.kafka_client&.close
end

# :nodoc:
def self.execute(producer_class:, messages:)
Deimos.instrument(
Expand Down
7 changes: 0 additions & 7 deletions lib/deimos/config/configuration.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# frozen_string_literal: true

require 'fig_tree'
require_relative 'phobos_config'
require_relative '../metrics/mock'
require_relative '../tracing/mock'
require 'active_support/core_ext/numeric'
Expand All @@ -10,14 +9,8 @@
module Deimos # rubocop:disable Metrics/ModuleLength
include FigTree

# :nodoc:
class FigTree::ConfigStruct
include Deimos::PhobosConfig
end

# :nodoc:
after_configure do
Phobos.configure(self.config.phobos_config)
if self.config.schema.use_schema_classes
load_generated_schema_classes
end
Expand Down
164 changes: 0 additions & 164 deletions lib/deimos/config/phobos_config.rb

This file was deleted.

1 change: 0 additions & 1 deletion lib/deimos/consume/batch_consumption.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ module Consume
# of messages to be handled at once
module BatchConsumption
extend ActiveSupport::Concern
include Phobos::BatchHandler
Copy link
Collaborator

Choose a reason for hiding this comment

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

There are quite a few references to Phobos on sig/defs.rbs file. I believe they needs to be removed too.


# @param batch [Array<String>]
# @param metadata [Hash]
Expand Down
1 change: 0 additions & 1 deletion lib/deimos/consume/message_consumption.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ module Consume
# are invoked for every individual message.
module MessageConsumption
extend ActiveSupport::Concern
include Phobos::Handler

# @param payload [String]
# @param metadata [Hash]
Expand Down
4 changes: 2 additions & 2 deletions lib/deimos/kafka_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,14 @@ def self.decoded(messages=[])
end
end

# @return [Hash]
def phobos_message
def karafka_message
{
payload: self.message,
partition_key: self.partition_key,
key: self.key,
topic: self.topic
}
end

end
end
35 changes: 0 additions & 35 deletions lib/deimos/monkey_patches/phobos_cli.rb

This file was deleted.

2 changes: 1 addition & 1 deletion lib/deimos/producer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@

require 'deimos/message'
require 'deimos/shared_config'
require 'phobos/producer'
require 'active_support/notifications'

# :nodoc:
module Deimos
class << self

# Run a block without allowing any messages to be produced to Kafka.
# Optionally add a list of producer classes to limit the disabling to those
# classes.
Expand Down
Loading