DRAFT: extract ciruit_options when using public call interface

[skorbut]

When providing circuit options while using the public call interface, these should be taken into account.

In order to do this we filter out known circuit options before merging all into the options.

Open question: Is there a definitive source for circuit_options keys?

Also I noticed, when using call with the circuit interface the circuit options are picked up, but get lost later in the process. See added specs in call_test.rb

[apotonick]

Hi Karsten,
thanks for looking into this! Have you seen this commit where I try to fix that very problem? ac2fa2b

There is no defined set for circuit_options, unfortunately!

[apotonick]

BTW, when using the public interface you're not supposed to provide circuit_options! The signature of Operation.call may suggest that, but you may only pass ctx and optional flow_options. Could you tell us why you need the public interface in combo with low-level options? 🍻

[skorbut]

Hi,

we currently have several operations that were implemented quite optimistically. In order to add more detail when reporting failed operations I'd like to supply a protocol that will upload to our error reporting tool. In this protocol I list all steps with its result and runtime. The protocol is created using a TaskWrap extension and added to every operation via some BaseOperation class were I override the call method like

def self.call(*args, **kwargs)
    puts "BaseOperation#call"
    # register protocol extension
    protocol_extension = Trailblazer::Activity::TaskWrap::Extension(
      [::OperationsProtocol.method(:start_task),  id: "protocol.start_task",  prepend: "task_wrap.call_task"],
      [::OperationsProtocol.method(:end_task), id: "protocol.end_task", append: "task_wrap.call_task"],
    )
    protocol_wrap = Hash.new(protocol_extension)
    ctx = kwargs || {}
    byebug
    #super([ctx, {}], wrap_runtime: protocol_wrap)
  end

I also tried switching from public interface to circuit interface here, but it didn't call the extension. It's like the spec I added in this PR for testing the circuit interface with wrap_runtime.

[apotonick]

This looks like you should definitely switch to the circuit interface: Operation.([{ctx}, {flow_options}], **your_options).

My plan has been (for a long time) to make as many users as possible invoke an endpoint that then calls the operation with appropriate options. The public Operation.call interface was a short-sighted idea to encourage service objects.

Can you check if the above signature works for you? Alternatively, try using the private method directly https://github.com/trailblazer/trailblazer-operation/blob/master/lib/trailblazer/operation/public_call.rb#L69

[apotonick]

This is - per design - not supposed to work as it's impossible for us to know which keyword argument is flow_options and what is circuit_options. It was a design mistake to try and make the public #call the swiss army knife of TRB...

[apotonick]

This should be working straight away: https://github.com/trailblazer/trailblazer-operation/pull/65/files#diff-bbbacc52ba06dba88cb878f1db7b74e5e5f0a5578c6482e9568d66af927315b2R139

[skorbut]

It should, but I'm afraid it doesn't. I tried to debug, where my wrap_runtime got lost. But got lost myself. Somehow it doesn't get called. Test is there to document this behaviour.

[skorbut]

Can you have a look at this spec? It is basically the same as the public interface spec, but it fails. Somehow the extension doesn't get called. If I get this to succeed, I wouldn't need the public interface change.

[apotonick]

Sorry I was busy with the new website, I will figure out where we lose your :wrap_runtime tomorrow!

[skorbut]

Thanks a lot, new Website looks great.

[apotonick]

@skorbut This is how you make it work when using Operation.call:

signal, (ctx, _) = operation.call(
      [{seq: []}, {}],
      wrap_runtime: Hash.new(my_extension),
      runner: Trailblazer::Activity::TaskWrap::Runner
    )

[apotonick]

Note that the ctx won't be automatically converted to a Context as it's done here: https://github.com/trailblazer/trailblazer-operation/blob/master/lib/trailblazer/operation/public_call.rb#L79

This is why I vote for slowly fading out usage of Operation.call in the next years in favor of invoking operations using an endpoint.

[skorbut]

Thanks a lot for looking into this, just ran a few tests and it looks good. I'm closing this PR, no need to change a deprecated interface.

[apotonick]

Well, technically, it's not deprecated as we cannot simply remove Create.(params: ...) 😬 the questions here are

1. how do we manage to get people use the circuit interface for low-level concerns?
2. what form of an endpoint would encourage developers to use that abstraction instead of globally overriding private methods to inject arbitrary data into the operation call?