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

Crash if search contains invalid substitution #86

Open
dmerejkowsky opened this issue Feb 13, 2021 · 2 comments
Open

Crash if search contains invalid substitution #86

dmerejkowsky opened this issue Feb 13, 2021 · 2 comments

Comments

@dmerejkowsky
Copy link
Collaborator

Steps to reproduce:

[[file]]
src = "..."
search = "{no-such-key}"

tbump crashes with:

:: Bumping from 2.5.1-1 to 2.5.2-1
Traceback (most recent call last):
  File "/mnt/data/dmerej/.local/bin/tbump", line 5, in <module>
    module.main()
  File "/mnt/data/dmerej/src/tanker/tbump/tbump/main.py", line 183, in main
    run(args)
  File "/mnt/data/dmerej/src/tanker/tbump/tbump/main.py", line 103, in run
    bump(bump_options, operations)
  File "/mnt/data/dmerej/src/tanker/tbump/tbump/main.py", line 137, in bump
    executor = Executor(new_version, file_bumper)
  File "/mnt/data/dmerej/src/tanker/tbump/tbump/executor.py", line 50, in __init__
    file_bumper.get_patches(new_version),
  File "/mnt/data/dmerej/src/tanker/tbump/tbump/file_bumper.py", line 198, in get_patches
    change_requests = self.compute_change_requests()
  File "/mnt/data/dmerej/src/tanker/tbump/tbump/file_bumper.py", line 241, in compute_change_requests
    change_request = self.compute_change_request_for_file(file)
  File "/mnt/data/dmerej/src/tanker/tbump/tbump/file_bumper.py", line 278, in compute_change_request_for_file
    to_search = file.search.format(current_version=re.escape(current_version))
KeyError: 'no-such-key
@plannigan
Copy link
Contributor

Something I noticed was that currently there are lots of places where .format(...) is called. This makes it tricky to track down all the places where that is called and it is hard to know what replacement keys can be used.

What would you think if I approached this with a bit of a bigger change? My idea is to have a single class that is only in charge of formatting these templates. This class would know the keys & values that could be used and only needs to be given the specific format template. This would make it easy to handle these errors in a consistent way so that the CLI can display the message. It also would standardize the replacement keys that are available and make it easy to add new ones if needed. (Could also address #109 and make it easier to achieve #85.)

This would move a lot of things around since new_version and current_version are currently passed into a lot of different places. So I wouldn't want to dive into that if it would be something that would be unlikely to be merged. Thought?

@dmerejkowsky
Copy link
Collaborator Author

So I wouldn't want to dive into that if it would be something that would be unlikely to be merged.

I think having a separate class is a really good idea -go for it ! We have plenty of tests so it's unlikely we get regressions - and even then we'll be able to fix them :)

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