Skip to content

Remove null values from configuration file. - #32

Merged
gouttegd merged 3 commits into
mainfrom
fix-spurious-null-values-in-config
Sep 22, 2026
Merged

gouttegd merged 3 commits into
mainfrom
fix-spurious-null-values-in-config

Conversation

@gouttegd

Copy link
Copy Markdown
Collaborator

Automatically generated configuration files (obtained by seeding a repository without a provided configuration file) contain explicit null values for all settings that do not have a default value and do not have been explicitly set on the command line.

Such values (i) may cause problems downstream of parsing the configuration, and (ii) needlessly pollute the configuration file.

We take care of both problems by

  • adding a purge_null_values method to explicitly clean a dictionary of any null values;
  • using that new method in two occasions:
    • when saving the configuration (so that an automatically generated or updated file never contains null values in the first place);
    • after parsing the configuration and before trying to update it (in case a file would still somehow contain null values).

closes #31

Automatically generated configuration files (obtained by seeding a
repository without a provided configuration file) contain explicit null
values for all settings that do not have a default value and do not have
been explicitly set on the command line.

Such values (i) may cause problems downstream of parsing the
configuration, and (ii) needlessly pollute the configuration file.

We take care of both problems by

* adding a `purge_null_values` method to explicitly clean a dictionary
  of any null values;
* using that new method in two occasions:
    * when saving the configuration (so that an automatically generated
      or updated file never contains null values in the first place);
    * after parsing the configuration and before trying to update it (in
      case a file would still somehow contain null values).

closes #31
@gouttegd gouttegd self-assigned this Sep 17, 2026
@gouttegd
gouttegd requested a review from matentzn September 17, 2026 12:29

@matentzn matentzn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, two optional thoughts

Comment thread src/incatools/odk/config.py Outdated
Comment thread src/incatools/odk/config.py
Amend the `purge_null_values` method so that it enters lists and purge
any null values in any dict that may be contained (directly or
indirectly) in a list item.
@gouttegd
gouttegd requested a review from matentzn September 19, 2026 19:45

@matentzn matentzn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing you should contemplate before merging, but I leave this up to your judgment:

null means "use the default" everywhere, and for a few options the default isn't empty. Concretely, a config with

ci:
workflows:

gives you ci = ['github_actions'] and workflows = ['docs', 'qc']

Since this works:

ci: []
workflows: []

I am fine to say: lets deal with this with docs; Your judgment.

@matentzn matentzn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry meant to approve with last comment

@gouttegd

Copy link
Copy Markdown
Collaborator Author

null means "use the default" everywhere, and for a few options the default isn't empty. > Concretely, a config with

ci:
workflows:

gives you ci = ['github_actions'] and workflows = ['docs', 'qc']

Actually such a config gives an error (just tested with ODK 1.6.1):

Traceback (most recent call last):
  File "/tools/odk.py", line 1511, in <module>
    cli()
  File "/usr/local/lib/python3.12/dist-packages/click/core.py", line 1161, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/dist-packages/click/core.py", line 1082, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/dist-packages/click/core.py", line 1697, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/dist-packages/click/core.py", line 1443, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/dist-packages/click/core.py", line 788, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tools/odk.py", line 1441, in seed
    tgts += install_template_files(mg, templatedir, outdir, policies)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tools/odk.py", line 1061, in install_template_files
    s.write(generator.generate(srcf))
            ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tools/odk.py", line 890, in generate
    return template.render( project = self.context.project, env = {"ODK_VERSION": os.getenv("ODK_VERSION")})
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/dist-packages/jinja2/environment.py", line 1295, in render
    self.environment.handle_exception()
  File "/usr/local/lib/python3.12/dist-packages/jinja2/environment.py", line 942, in handle_exception
    raise rewrite_traceback_stack(source=source)
  File "<template>", line 1, in top-level template code
TypeError: argument of type 'NoneType' is not iterable

So since using empty values (either implicitly, as in ci:, or explicitly, as in ci: null) is not in fact currently supported, we are not loosing anything.

Without this PR:

  • if you want the default, just omit the key entirely;
  • if you want an empty list of workflows, explicitly set an empty list (workflows: [])
  • if you set the key to nothing (workflows: or workflows: null), you get the error above.

With this PR:

  • if you want the default, either omit the key entirely or set it to nothing;
  • if you want an empty list of workflows, explicitly set an empty list.

@gouttegd
gouttegd merged commit a1800f8 into main Sep 22, 2026
6 checks passed
@gouttegd
gouttegd deleted the fix-spurious-null-values-in-config branch September 22, 2026 15:34
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

Successfully merging this pull request may close these issues.

Null values in configuration cause issues

2 participants