Skip to content

Commit

Permalink
Fix warnings that started popping up when running on ruby 3 (#441)
Browse files Browse the repository at this point in the history
* Fix warnings that started popping up when running on ruby 3

These were fixed
lib/dynflow/coordinator.rb:149: warning: method redefined; discarding old to_s
lib/dynflow/coordinator.rb:133: warning: previous definition of to_s was here
lib/dynflow/persistence_adapters/sequel.rb:102: warning: assigned but unused variable - output_chunks
lib/dynflow/action/timeouts.rb:13: warning: mismatched indentations at 'end' with 'module' at 3
lib/dynflow/executors/parallel.rb:27: warning: assigned but unused variable - error
lib/dynflow/executors/parallel/pool.rb:27: warning: method redefined; discarding old queue_size
lib/dynflow/executors/parallel/pool.rb:19: warning: previous definition of queue_size was here
lib/dynflow/world.rb:300: warning: assigned but unused variable - e
lib/dynflow/telemetry.rb:21: warning: `&' interpreted as argument prefix
lib/dynflow/testing/mimic.rb:33: warning: `&' interpreted as argument prefix
lib/dynflow/testing/mimic.rb:35: warning: `&' interpreted as argument prefix
lib/dynflow/testing/factories.rb:42: warning: `*' interpreted as argument prefix
lib/dynflow/persistence_adapters/sequel_migrations/018_add_uuid_column.rb:2: warning: method redefined; discarding old to_uuid
lib/dynflow/persistence_adapters/sequel_migrations/018_add_uuid_column.rb:2: warning: previous definition of to_uuid was here
lib/dynflow/persistence_adapters/sequel_migrations/018_add_uuid_column.rb:6: warning: method redefined; discarding old from_uuid
lib/dynflow/persistence_adapters/sequel_migrations/018_add_uuid_column.rb:6: warning: previous definition of from_uuid was here
lib/dynflow/persistence_adapters/sequel_migrations/018_add_uuid_column.rb:10: warning: method redefined; discarding old with_foreign_key_recreation
lib/dynflow/persistence_adapters/sequel_migrations/018_add_uuid_column.rb:10: warning: previous definition of with_foreign_key_recreation was here

The following three come from us dynamically generating methods and then overriding some of them
lib/dynflow/config.rb:105: warning: method redefined; discarding old validate_executor!
lib/dynflow/config.rb:9: warning: previous definition of validate_executor! was here
lib/dynflow/testing/mimic.rb:15: warning: method redefined; discarding old ===

* Pin bundler in CI

* Fix warnings from 018 migration differently
  • Loading branch information
adamruzicka authored Jan 29, 2024
1 parent b2a4a57 commit 150a23b
Show file tree
Hide file tree
Showing 11 changed files with 46 additions and 48 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ruby.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ jobs:
ruby-version: 2.7
- name: Setup
run: |
gem install bundler
gem install bundler -v 2.4.22
bundle install --jobs=3 --retry=3
- name: Run rubocop
run: bundle exec rubocop
Expand Down
2 changes: 1 addition & 1 deletion lib/dynflow/action/timeouts.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@ def process_timeout
def schedule_timeout(seconds, optional: false)
plan_event(Timeout, seconds, optional: optional)
end
end
end
end
4 changes: 0 additions & 4 deletions lib/dynflow/coordinator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -145,10 +145,6 @@ def validate!
raise "Can't acquire the lock after deserialization" if @from_hash
Type! owner_id, String
end

def to_s
"#{self.class.name}: #{id} by #{owner_id}"
end
end

class LockByWorld < Lock
Expand Down
2 changes: 1 addition & 1 deletion lib/dynflow/executors/parallel.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def execute(execution_plan_id, finished = Concurrent::Promises.resolvable_future
accepted = @core.ask([:handle_execution, execution_plan_id, finished])
accepted.value! if wait_for_acceptance
finished
rescue Concurrent::Actor::ActorTerminated => error
rescue Concurrent::Actor::ActorTerminated
dynflow_error = Dynflow::Error.new('executor terminated')
finished.reject dynflow_error unless finished.resolved?
raise dynflow_error
Expand Down
4 changes: 0 additions & 4 deletions lib/dynflow/executors/parallel/pool.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ def pop
@jobs.shift
end

def queue_size
execution_status.values.reduce(0, :+)
end

def empty?
@jobs.empty?
end
Expand Down
2 changes: 1 addition & 1 deletion lib/dynflow/persistence_adapters/sequel.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ def delete_execution_plans(filters, batch_size = 1000, backup_dir = nil)
backup_to_csv(:step, steps, backup_dir, 'steps.csv') if backup_dir
steps.delete

output_chunks = table(:output_chunk).where(execution_plan_uuid: uuids).delete
table(:output_chunk).where(execution_plan_uuid: uuids).delete

actions = table(:action).where(execution_plan_uuid: uuids)
backup_to_csv(:action, actions, backup_dir, 'actions.csv') if backup_dir
Expand Down
Original file line number Diff line number Diff line change
@@ -1,45 +1,49 @@
# frozen_string_literal: true
def to_uuid(table_name, column_name)
set_column_type(table_name, column_name, :uuid, :using => "#{column_name}::uuid")
end

def from_uuid(table_name, column_name)
set_column_type table_name, column_name, String, primary_key: true, size: 36, fixed: true
end

def with_foreign_key_recreation(&block)
# Drop the foreign key constraints so we can change the column type
alter_table :dynflow_actions do
drop_foreign_key [:execution_plan_uuid]
end
alter_table :dynflow_steps do
drop_foreign_key [:execution_plan_uuid]
drop_foreign_key [:execution_plan_uuid, :action_id], :name => :dynflow_steps_execution_plan_uuid_fkey1
helper = Module.new do
def to_uuid(table_name, column_name)
set_column_type(table_name, column_name, :uuid, :using => "#{column_name}::uuid")
end
alter_table :dynflow_delayed_plans do
drop_foreign_key [:execution_plan_uuid]

def from_uuid(table_name, column_name)
set_column_type table_name, column_name, String, primary_key: true, size: 36, fixed: true
end

block.call
def with_foreign_key_recreation(&block)
# Drop the foreign key constraints so we can change the column type
alter_table :dynflow_actions do
drop_foreign_key [:execution_plan_uuid]
end
alter_table :dynflow_steps do
drop_foreign_key [:execution_plan_uuid]
drop_foreign_key [:execution_plan_uuid, :action_id], :name => :dynflow_steps_execution_plan_uuid_fkey1
end
alter_table :dynflow_delayed_plans do
drop_foreign_key [:execution_plan_uuid]
end

# Recreat the foreign key constraints as they were before
alter_table :dynflow_actions do
add_foreign_key [:execution_plan_uuid], :dynflow_execution_plans
end
alter_table :dynflow_steps do
add_foreign_key [:execution_plan_uuid], :dynflow_execution_plans
add_foreign_key [:execution_plan_uuid, :action_id], :dynflow_actions,
:name => :dynflow_steps_execution_plan_uuid_fkey1
end
alter_table :dynflow_delayed_plans do
add_foreign_key [:execution_plan_uuid], :dynflow_execution_plans,
:name => :dynflow_scheduled_plans_execution_plan_uuid_fkey
block.call

# Recreat the foreign key constraints as they were before
alter_table :dynflow_actions do
add_foreign_key [:execution_plan_uuid], :dynflow_execution_plans
end
alter_table :dynflow_steps do
add_foreign_key [:execution_plan_uuid], :dynflow_execution_plans
add_foreign_key [:execution_plan_uuid, :action_id], :dynflow_actions,
:name => :dynflow_steps_execution_plan_uuid_fkey1
end
alter_table :dynflow_delayed_plans do
add_foreign_key [:execution_plan_uuid], :dynflow_execution_plans,
:name => :dynflow_scheduled_plans_execution_plan_uuid_fkey
end
end
end

Sequel.migration do
up do
if database_type.to_s.include?('postgres')
Sequel::Postgres::Database.include helper

with_foreign_key_recreation do
to_uuid :dynflow_execution_plans, :uuid
to_uuid :dynflow_actions, :execution_plan_uuid
Expand All @@ -51,6 +55,8 @@ def with_foreign_key_recreation(&block)

down do
if database_type.to_s.include?('postgres')
Sequel::Postgres::Database.include helper

with_foreign_key_recreation do
from_uuid :dynflow_execution_plans, :uuid
from_uuid :dynflow_actions, :execution_plan_uuid
Expand Down
2 changes: 1 addition & 1 deletion lib/dynflow/telemetry.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def set_adapter(adapter)
# Passes the block into the current telemetry adapter's
# {TelemetryAdapters::Abstract#with_instance} method
def with_instance(&block)
@instance.with_instance &block
@instance.with_instance(&block)
end

def measure(name, tags = {}, &block)
Expand Down
2 changes: 1 addition & 1 deletion lib/dynflow/testing/factories.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def create_action_presentation(action_class)
def plan_action(plan_action, *args, &block)
Match! plan_action.phase, Action::Plan

plan_action.execute *args, &block
plan_action.execute(*args, &block)
raise plan_action.error if plan_action.error
plan_action
end
Expand Down
4 changes: 2 additions & 2 deletions lib/dynflow/testing/mimic.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ def mimic!(*types)
end

if self.kind_of? ::Class
self.class_eval &define
self.class_eval(&define)
else
self.singleton_class.class_eval &define
self.singleton_class.class_eval(&define)
end

self
Expand Down
2 changes: 1 addition & 1 deletion lib/dynflow/world.rb
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ def try_spawn(what, lock_class = nil)
coordinator.acquire(lock_class.new(self)) if lock_class
object.spawn.wait
object
rescue Coordinator::LockError => e
rescue Coordinator::LockError
nil
end

Expand Down

0 comments on commit 150a23b

Please sign in to comment.