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

DM-27638: Try to ensure that previous module not found error is included #91

Merged
merged 1 commit into from Nov 20, 2020

Conversation

timj
Copy link
Member

@timj timj commented Nov 19, 2020

Since doImport treats ModuleNotFoundError as a special error
indicating that it should probably retry with a different
subset of the supplied string, this means that failures
to import dependencies trigger a confusing error report.
It's hard to disentangle real errors from dependencies
versus a request for a class within a module, so as a short
term fix include the previous import error in the final
error. Hopefully this will mitigate the problem.

Also add some explicit tests for this scenario.

Since doImport treats ModuleNotFoundError as a special error
indicating that it should probably retry with a different
subset of the supplied string, this means that failures
to import dependencies trigger a confusing error report.
It's hard to disentangle real errors from dependencies
versus a request for a class within a module, so as a short
term fix include the previous import error in the final
error. Hopefully this will mitigate the problem.

Also add some explicit tests for this scenario.
Copy link

@MichelleGower MichelleGower left a comment

Choose a reason for hiding this comment

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

Code changes looked good. Tested it and got the more helpful error message.

@timj timj merged commit d222c45 into master Nov 20, 2020
@timj timj deleted the tickets/DM-27638 branch November 20, 2020 00:10
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.

None yet

2 participants