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

Deprecation message spam #143

Open
gshively11 opened this issue Feb 23, 2022 · 6 comments
Open

Deprecation message spam #143

gshively11 opened this issue Feb 23, 2022 · 6 comments

Comments

@gshively11
Copy link

Hey folks, I'm just dipping my toes into temporal and this SDK is a lifesaver, thanks for open sourcing it!

I'm playing with temporal locally and I'm seeing a lot of deprecation message spam when running workflows/activities:

temporal-poc-app-1      | [DEPRECATION] This method is now deprecated without a substitution
temporal-poc-app-1      | [DEPRECATION] This method is now deprecated without a substitution
temporal-poc-app-1      | [DEPRECATION] This method is now deprecated without a substitution
temporal-poc-app-1      | [DEPRECATION] This method is now deprecated without a substitution
temporal-poc-app-1      | [DEPRECATION] This method is now deprecated without a substitution

This appears to be due to this line.

Which is still used in multiple spots in the codebase:

Do you have any guidance for removing the deprecated code? I'm happy to attempt a PR, but wanted to check with the maintainers first.

@DeRauk
Copy link
Contributor

DeRauk commented Feb 24, 2022

Hi Grant, glad you're enjoying the SDK!

The 2nd link you gave should be fairly simple to refactor, just replace Temporal.configuration.namespace with @config.namespace.

For the third link you don’t need to refactor anything, just pass in a configuration object instead of using the default option when creating your worker.

The first link requires a pretty big refactor if I remember right, I’ll let @antstorm chime in on how to approach that one.

@antstorm
Copy link
Contributor

@gshively11 agree with what DeRauk said above. The first link is definitely a bigger refactor project, but should also be doable. I think the best approach there is to make payload converters (Temporal::Concerns::Payloads) part of the configuration instead of a mixin and then ensure that configuration is propagated where needed.

We can also avoid breaking changes by having one or two places where we fallback onto default configuration if nothing else is provided, but it should be at the top level and would result in minimal output noise.

I'm happy to assist you with the PR or I can take a look at it next week in case you feel like it might be a bit too much.

@gshively11
Copy link
Author

Thanks for chiming in @DeRauk and @antstorm. I can give the PR a try, although it might be weeks before I get around to it due to other responsibilities. If this is something you (or anyone) wants to fix sooner, feel free to grab it, just drop a comment letting me know.

@antstorm
Copy link
Contributor

@gshively11 I might have some time to pick it up this week. Will ping you on the PR

@bobbytables
Copy link

For other travelers wondering how to silence this, I ended up not caring about this message (it has been 'deprecated' for over 2 years now, afterall).

I added this to my config/initializer/temporal.rb file in my rails app:

def Temporal.warn(msg)
  Rails.logger.warn(msg)
end

Voilá - no more warnings.

(I know this is bad, I just couldnt take it anymore)

@aguynamedben
Copy link

First, thank you so much for this gem, it's incredible. 🙏

I'm also not sure if I'm "doing it wrong". I'm just following the README but seeing deprecation warnings when starting a worker with a Rails initializer configured. It seems like some code internal to temporal-ruby is calling Temporal#configuration, but I'm only calling Temporal#configure as the README directs.

It seems most of the deprecation warnings might have been cleaned up in the past few years and the only one left is this one? https://github.com/search?q=repo%3Acoinbase%2Ftemporal-ruby%20This%20method%20is%20now%20deprecated&type=code

Maybe we can remove the warning or remove the method?

config/initializers/temporal.rb

require "temporal/client"
require "temporal/worker"

Temporal.configure do |config|
  config.host = ENV.fetch("TEMPORAL_HOST", "localhost")
  config.port = ENV.fetch("TEMPORAL_PORT", 7233)
  config.namespace = ENV.fetch("TEMPORAL_NAMESPACE", "hui")
  config.task_queue = "default"
end

lib/tasks/temporal.rake

namespace :temporal do
  desc "Start the Temporal worker process"
  task worker: :environment do
    TemporalWorker.start
  end
end

app/workers/temporal_worker.rb

class TemporalWorker
  class << self
    def start
      # Initialize the worker
      worker = Temporal::Worker.new

      # Register all workflows
      worker.register_workflow(MyWorkflow)

      # Register all activities
      worker.register_activity(MyActivity)

      # Start the worker - this will take over the process and keep running
      # until a TERM or INT signal is received
      Rails.logger.info "Starting Temporal worker..."
      worker.start
    end
  end
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

5 participants