-
Notifications
You must be signed in to change notification settings - Fork 56
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
Soften error to warning in template processor when fails to read file #1924
Conversation
|
||
if expanded_template != file.contents: | ||
file.edited_contents = expanded_template | ||
except UnicodeDecodeError as err: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why would we get this error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it makes sense for binary files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So that's where those errors are coming from 🤯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sfc-gh-mchok you saw these on the dashboard?! Yeah our template processor was dying every time there was a file it couldn't read :D
|
||
if expanded_template != file.contents: | ||
file.edited_contents = expanded_template | ||
except UnicodeDecodeError as err: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it makes sense for binary files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No test?
bundle_result = bundle_files(tmp_dir, file_name, file_contents) | ||
templates_processor = TemplatesProcessor(bundle_ctx=bundle_result.bundle_ctx) | ||
with mock.patch( | ||
f"{ARTIFACT_PROCESSOR}.ProjectFileContextManager.__enter__", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not just mock edit_file
? That's the public contract, whereas this is an implementation detail.
Pre-review checklist
Changes description
Template processor should not fail if it can't read a file.