Skip to content

--config-value with a JSON array, number, or string crashes with an internal error #3390

Open

Description

@eyosi1239

Passing a config value that's valid JSON but not an object crashes with
an internal error instead of a config error. Each exits with status 1:

--config-value cfg Result
'[]' AttributeError: 'list' object has no attribute 'items'
'1' AttributeError: 'int' object has no attribute 'items'
'"x"' AttributeError: 'str' object has no attribute 'items'
python flow.py --config-value cfg '[]' show

Cause

JSON parsing in metaflow/user_configs/config_options.py (~line 388)
succeeds, then ConfigValue in
metaflow/user_configs/config_parameters.py (~line 85) calls .items()
on the result without checking its type.

Expected

A clear config error saying the value for cfg must be a JSON object,
consistent with how other malformed config values are reported.
null currently works through a separate None path and should keep
working as it does today.

Related

#3075 adds a root-type check for its YAML fallback, but that only runs
after JSON parsing fails, so it doesn't cover these cases.

Proposed fix

After json.loads, reject any non-None result that isn't a dict with a
config error, preserving the existing null behavior. Add parametrized
tests for arrays, numbers, strings, and null.

Since this touches user_configs/ (Core Runtime), I wanted to confirm
the approach before opening a PR.

Environment

  • OS: macOS Tahoe 26.5.2
  • Python: 3.12.14
  • Metaflow: master (8b500daa)

Activity

  1. changed the title [-]--config-value with a JSON array, number, or string crashes with an internal error`[/-] [+]--config-value with a JSON array, number, or string crashes with an internal error[/+] on Sep 29, 2026
  2. suhanemathur commented on Sep 29, 2026

    @suhanemathur

    Hi @eyosi1239 , I'd like to take this issue up.

  3. eyosi1239 commented on Sep 29, 2026

    @eyosi1239
    Author

    Thanks for the interest, @suhanemathur! I found this while testing and
    already have the approach and tests worked out locally, so I'd like to
    take it. Per CONTRIBUTING.md I'm just waiting on maintainer confirmation
    before opening a PR. @saikonen would you be okay with me picking this
    up?

  4. saikonen commented on Sep 29, 2026

    @saikonen
    Collaborator

    seems like a reasonable bug fix. When tackling this, be sure to only cover the JSON path, and maybe not specifically mention JSON in the error messages as this can also result from parsing other types with custom parsers.

    Note that bare str configs are also valid:

    from metaflow import FlowSpec, step, Config, environment
    import os
    
    class ConfigTestFlow(FlowSpec):
        barecfg = Config('barecfg', default_value="", plain=True)
    
        @step
        def start(self):
            print("BareCFG: ", self.barecfg)
            self.next(self.end)
    
        @step
        def end(self):
            pass
    
    if __name__ == '__main__':
        ConfigTestFlow()
    python configflow.py --config-value barecfg "a bare testvalue" run
    
  5. eyosi1239 commented on Sep 29, 2026

    @eyosi1239
    Author

    opened #3391 with the fix. @saikonen could you take a look and approve the workflow run when you get a chance?

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

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions