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

Job arg Symbol conversion is complicated #427

Open
owst opened this issue Jul 9, 2024 · 1 comment
Open

Job arg Symbol conversion is complicated #427

owst opened this issue Jul 9, 2024 · 1 comment

Comments

@owst
Copy link
Contributor

owst commented Jul 9, 2024

Que >= 1 is fairly mature at this point, but we're only now looking to migrate away from Que 0.X. For job arguments that contain Symbols, round-tripping is complicated in Que >= 1. For a simple job, enqueued with complex arguments:

class TestJob < Que::Job
  def run(*args)
    args.each { |a| p a }
  end
end

TestJob.enqueue(
  { foo: { bar: :baz } },
  { "foo" => { "bar" => "baz" } },
  [ { "inside" => { "an" => "array" } }],
  ["array string"],
  [:array_sym],
  "a string",
  :a_sym,
  42
)

with Que 0.14.3 we see:

{"foo"=>{"bar"=>"baz"}}
{"foo"=>{"bar"=>"baz"}}
[{"inside"=>{"an"=>"array"}}]
["array string"] 
["array_sym"]
"a string"
"a_sym"
42

whereas with Que 2.3.0:

{:foo=>{:bar=>"baz"}}
{:foo=>{:bar=>"baz"}}
[{:inside=>{:an=>"array"}}]
["array string"]
["array_sym"]
"a string"
"a_sym"
42

For Que 0.X the "conversion rule" was simple: "all Symbol values become Strings". However, for Que >=1 the rule is complicated: Hash keys (even when nested or within Arrays) are converted to Symbols (regardless of their original class) but other Symbols are converted to Strings, and other Strings remain as Strings.

This makes transitioning from Que 0.X to Que >= 1 more difficult as existing jobs expect that args will not contain Symbols. It seems more obvious that round-tripping an arbitrary Hash via JSON will stringify (all) Symbols and thus inconsistent for (e.g.) { "foo" => { "bar" => "baz" } } to become {:foo=>{:bar=>"baz"}} but [:array_sym] to become ["array_sym"] in job arguments.

So some questions:

  1. Why was symbolize_names: true used, given that it leads to inconsistent treatment of Symbols?
  2. There used to be functionality to support an alternative JSON deserializer (removed in 76c3cab) - could this be reinstated? Or at least allow symbolize_names to be configurable
@ZimbiX
Copy link
Member

ZimbiX commented Jul 24, 2024

Interesting. This does sound unexpected! And rather a breaking change to fix..

I think symbolize_names: true was probably added to allow jobs to use keyword arguments (perhaps without realising the impact on keys in all parts of the structure). This would not work if the keys at the top level of the hash remained strings. Perhaps Que should be changed to only symbolise the keys in top-level hash of the kwargs.

Is there another reason why someone might want to customise the JSON deserialiser? I wonder if adding a setting on a job might be a more targeted way to address this, and we could then deprecate the old behaviour. Although that approach would be less flexible.

I do not envy your journey of migrating from Que 0.x - I never used it myself, and know very little about it.

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

No branches or pull requests

2 participants