Pass ruby objects between states

[kbrock]

Overview

See also #228

Built upon:

• Output should be in json #230

I am displaying these opinions so we can more easily discus them:

• We need better error messaging around bad input and context, specifically handling JSON::ParserError.
• Who converts input from JSON to ruby? CLI and Context.load, Context.from_json (will be used in the provider)
• Should the Context#input hold JSON or ruby? ruby
• Should the Context#output hold JSON or ruby? ruby
• Should Context in the database be JSON or ruby? ruby, the database serializes it for us.

---

• floe/exe is responsible for converting user strings to ruby/json objects.
• Task#parse_output is responsible for converting task output to ruby/json objects.
• All classes behind that barrier deal with ruby objects.
• remove JSON.parse(x) if x.kind_of?(String)

[kbrock]

update:

• rebase
• removed context changes and kept with input only

[agrare]

@kbrock I think we could use some specs covering using simple strings not hashes as input/output

I applied this to #184 and I'm seeing

  1) Floe::Workflow::States::Map#run_nonblock! has no next
     Failure/Error: output&.key?("Error") || false
     
     NoMethodError:
       undefined method `key?' for "R31":String
     
               output&.key?("Error") || false
                     ^^^^^^
     # ./lib/floe/workflow/context.rb:39:in `failed?'
     # ./lib/floe/workflow/state.rb:67:in `finish'
     # ./lib/floe/workflow/states/non_terminal_mixin.rb:11:in `finish'
     # ./lib/floe/workflow/states/pass.rb:30:in `finish'
     # ./lib/floe/workflow/state.rb:51:in `run_nonblock!'
     # ./lib/floe/workflow_base.rb:32:in `step_nonblock'
     # ./lib/floe/workflow_base.rb:25:in `run_nonblock'
     # ./lib/floe/workflow/states/map.rb:90:in `step'
     # ./lib/floe/workflow/states/map.rb:57:in `start'
     # ./lib/floe/workflow/state.rb:48:in `run_nonblock!'
     # ./spec/workflow/states/map_spec.rb:110:in `block (3 levels) in <top (required)>'

This is fixed by my changes https://github.com/ManageIQ/floe/pull/214/files#diff-c61a9c0cebe305116e077d04dc168aa33f6339d348c778addf65ab7c19b2479bR42 and https://github.com/ManageIQ/floe/pull/214/files#diff-24e71eb9cd8cfb4211a041b7a08cfc1d9d33203a6c2ce88dbc153234e82dfe1cR67

[kbrock]

@agrare thanks for the pointer on this one
I was getting the same errors
I merged 184 and they seem to be resolved

[Fryguy]

Had a question here about the intent. I noticed a comment in #214 where we are expecting "non-json" output, but to clarify, we really are expecting "non-hash" output. Well-formed JSON strings or integers are still JSON even if they aren't a Hash. So my concern here if we don't attempt to JSON parse it, is that invalid outputs can be emitted and we would miss verifying them.

So, here we see valid JSON "bare" strings and integers:

[1] pry(main)> require "json"
=> true
[2] pry(main)> "test".to_json
=> "\"test\""
[3] pry(main)> JSON.parse(_)
=> "test"
[4] pry(main)> 1.to_json
=> "1"
[5] pry(main)> JSON.parse(_)
=> 1

But this would be invalid output/input:

[2] pry(main)> JSON.parse("foo")
~/.gem/ruby/3.1.5/gems/json-2.7.2/lib/json/common.rb:220:in `parse': unexpected token at 'foo' (JSON::ParserError)
	from ~/.gem/ruby/3.1.5/gems/json-2.7.2/lib/json/common.rb:220:in `parse'
	from (pry):2:in `__pry__'
[3] pry(main)> JSON.parse('{\"xxx":')
~/.gem/ruby/3.1.5/gems/json-2.7.2/lib/json/common.rb:220:in `parse': unexpected token at '{\"xxx":' (JSON::ParserError)
	from ~/.gem/ruby/3.1.5/gems/json-2.7.2/lib/json/common.rb:220:in `parse'
	from (pry):3:in `__pry__'

[Fryguy]

For example. I think this is actually invalid input, right?

[kbrock]

it is invalid input for CLI.

In the CLI / floe from the command line, I want to see "\"foo\"".
And possibly the methods for manageiq-providers-workflow would pass in JSON strings.
But once something is in our inner classes, I want to see ruby.

[kbrock]

update:

• rebased
• added in checking at command line

outstanding:

• ensure json is in output (I see a lot of ruby e.g.: {"foo" => 1})

[kbrock]

update:

• more explicit error messages (fixed this)
• rubocop: fixed error catching complexity

[kbrock]

un-wip: we are now outputting in json (thanks to other PR)

[miq-bot]

Checked commits kbrock/floe@9262ef5~...3dab1ba with ruby 3.1.5, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
5 files checked, 0 offenses detected
Everything looks fine. 👍

[miq-bot]

This pull request is not mergeable. Please rebase and repush.

[kbrock]

We've decided to do json in the context.