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

Parsing config with url containg '%' has interpolation issue from ConfigParser #1018

Open
pedrohc opened this issue Nov 3, 2023 · 4 comments

Comments

@pedrohc
Copy link

pedrohc commented Nov 3, 2023

I'm using the Bugzilla service with the bugzilla.query_url option like this in .bugwarrionrc:

bugzilla.query_url = https://bugzilla.redhat.com/buglist.cgi?bug_status=NEW&bug_status=ASSIGNED&columnlist=product%2Ccomponent%2Cassigned_to%2Cstatus%2Csummary[...rest omitted...]

And get the following error while running $bugwarrior-pull --flavor bz.task:

ERROR:bugwarrior.services:Worker for [flavor.bz.task] failed: '%' must be followed by '%' or '(', found: '%2Ccomponent%2Cassigned_to%2Cstatus%2Csummary[omitted]
Traceback (most recent call last):
  File "/.../lib/python3.11/site-packages/bugwarrior/services/__init__.py", line 502, in _aggregate_issues
    service = get_service(service_name)(conf, main_section, target)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/.../lib/python3.11/site-packages/bugwarrior/services/bz.py", line 135, in __init__
    self.query_url = self.config.get('query_url', default=None)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/.../lib/python3.11/site-packages/bugwarrior/config.py", line 312, in get
    value = self.config_parser.get(self.service_target, self._get_key(key))
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib64/python3.11/configparser.py", line 815, in get
    return self._interpolation.before_get(self, section, option, value,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib64/python3.11/configparser.py", line 396, in before_get
    self._interpolate_some(parser, option, L, value, section, defaults, 1)
  File "/usr/lib64/python3.11/configparser.py", line 443, in _interpolate_some
    raise InterpolationSyntaxError(
configparser.InterpolationSyntaxError: '%' must be followed by '%' or '(', found: '%2Ccomponent%2Cassigned_to%2Cstatus%2Csummary[omitted]
INFO:bugwarrior.services:Done with [flavor.bz.task] in 0.005113s
INFO:bugwarrior.services:Terminating workers
ERROR:bugwarrior.command:Aborted (critical error in target 'flavor.bz.task')
@pedrohc
Copy link
Author

pedrohc commented Nov 3, 2023

https://github.com/ralphbean/bugwarrior/blob/dd96577c77949c8f07852785a8c8def368391970/bugwarrior/config.py#L232C98-L232C98

Had to include interpolation=None argument there to fix this. Can we make it permanent?

@ryneeverett
Copy link
Collaborator

https://github.com/ralphbean/bugwarrior/blob/dd96577c77949c8f07852785a8c8def368391970/bugwarrior/config.py#L232C98-L232C98

Had to include interpolation=None argument there to fix this. Can we make it permanent?

Note that one option is to work around this by escaping each instance of the percent sign with a double-percent sign. % -> %%

That said, I'd be inclined to agree that your proposal is a good idea. Technically it is a breaking change but that ship has mostly sailed since the last release. Some context:

  • Since the last release I've implemented strict validation that will not permit arbitrary variables. If somebody were using same-section interpolation this would be broken, but they could still use interpolation with the [DEFAULT] section.
  • Even though I've taken pains to minimize breaking changes, practically speaking everybody is going to have to update their configuration files for the next release anyway because it is next to impossible to write even a very small configuration file that is valid without automated validation.
  • Since the last release I've implemented TOML configuration as an alternative, which does not have this interpolation support. I don't think we should encourage use of features like interpolation that are specific to a single configuration language.
  • The only use case I've seen presented for interpolation in bugwarrior configuration is to reuse secrets, but I don't think we should encourage writing plaintext secrets in the config file anyway.

@pedrohc
Copy link
Author

pedrohc commented Nov 4, 2023

Agreed with your points there.

I tried escaping the '%' characters with another '%' to no success.

Also, tried using TOML syntax but was having other unrelated parsing errors.

So that was my last alternative which fixed the issue without too much complex tinkering with the code.

IMHO, if there are only few use cases for interpolation, a better way would be to make it contained to those special cases if its really needed. But I'm more inclined to your third point there.

Thanks for reviewing this. If you want me to make a PR, please let me know.

@ryneeverett
Copy link
Collaborator

Also, tried using TOML syntax but was having other unrelated parsing errors.

Please open an issue if you ran into bugs with the TOML syntax.

Thanks for reviewing this. If you want me to make a PR, please let me know.

Go for it!

@ryneeverett ryneeverett added this to the release-next milestone Feb 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants