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

Typed input and output #244

Open
arrowcircle opened this issue May 25, 2023 · 2 comments
Open

Typed input and output #244

arrowcircle opened this issue May 25, 2023 · 2 comments

Comments

@arrowcircle
Copy link

Hi! I am trying to get rid of JSON.parse(input) and JSON.dump(output) in activities/workflows and wanted to define Input and Output classes with dry-struct to define the structure.

Then I found this line in https://github.com/coinbase/temporal-ruby/blob/master/lib/temporal/concerns/typed.rb#L22

unless klass.is_a?(Dry::Types::Type)

Now I am stuck on how to make reusable structs with validation for workflow/activity params. I have three questions:

  1. Does it make sense to change condition in typed.rb to allow dry structs?
  2. How I can add typed output, so I don't need to serialize and deserialize outputs multiple time?
  3. Any best practices to validate inputs before activating the workflow?

Any ideas how to solve this?

@arrowcircle
Copy link
Author

For now I monkeypatched typed.rb with

module Temporal
  module Concerns
    module Typed
      module ClassMethods
        attr_reader :output_class

        def execute_in_context(context, input)
          if input_class
            input = input.first if input.is_a?(Array) && input.size == 1
            input = input_class.new(input)
          end
          output = super(context, input)
          return output unless output_class
          output_class.new(output)
        end

        def input(klass = nil, &block)
          if klass
            unless klass.is_a?(Dry::Types::Type) || klass << Dry::Struct
              raise 'Unsupported input class. Use one of the provided Temporal::Types'
            end
            @input_class = klass
          else
            @input_class = generate_struct
            @input_class.instance_eval(&block)
          end
        end

        def output(klass = nil, &block)
          if klass
            unless klass.is_a?(Dry::Types::Type) || klass << Dry::Struct
              raise 'Unsupported output class. Use one of the provided Temporal::Types'
            end
            @output_class = klass
          else
            @output_class = generate_struct
            @output_class.instance_eval(&block)
          end
        end

        private

        def generate_struct
          # TODO: Get rid of deprecation warning
          Class.new(Dry::Struct::Value) { transform_keys(&:to_sym) }
        end
      end
    end
  end
end

Would it be good to create PR with this?

@cdimitroulas
Copy link

Why do you need JSON.parse/JSON.dump in activities and workflows? There shouldn't be any need to do that 🤔

If you return hashes from activities/workflows, those are already handled automatically by the library. We simply use hashes (with string keys instead of symbols) for all activity inputs/outputs and it works fine.

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

No branches or pull requests

2 participants