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

[WIP] Improving error handling messages for Custom Functions #1330

Merged
merged 9 commits into from
Nov 6, 2023

Conversation

hershd23
Copy link
Contributor

@hershd23 hershd23 commented Nov 1, 2023

Added separate error handling for ModuleNotFoundError and FileNotFoundError
modified: evadb/utils/generic_utils.py

FileNotFoundError
	modified:   evadb/utils/generic_utils.py
	modified:   evadb/utils/generic_utils.py
@hershd23
Copy link
Contributor Author

hershd23 commented Nov 1, 2023

Hey @pchunduri6 @jarulraj, have added a couple of types of exceptions. Do let me know if there are other exceptions that we have to raise in difference places, will take care of that in this repository and add the tests as well

@pchunduri6
Copy link
Contributor

Error messages LGTM. Can you check the failed test cases?

@pchunduri6 pchunduri6 linked an issue Nov 1, 2023 that may be closed by this pull request
@hershd23
Copy link
Contributor Author

hershd23 commented Nov 2, 2023

Both the linter and the short integration test issues seem not to be linked to this issue in particular. I'll fix them nonetheless

…ode)

	modified:   evadb/binder/function_expression_binder.py
	modified:   test/integration_tests/short/test_select_executor.py
@jarulraj
Copy link
Member

jarulraj commented Nov 2, 2023

@hershd23 Have we already added unit test cases with functions with specifically these errors -- a function with only a module error and a function with only a file not found error and one with both errors?

@xzdandy xzdandy added this to the v0.3.9 milestone Nov 2, 2023
@hershd23
Copy link
Contributor Author

hershd23 commented Nov 3, 2023

FileNotFound - The existing short integration test was infact FileNotFound error
ModuleNotFound - It is a little unclear to me as to how to create such a test @xzdandy @gaurav274 any ideas?

@xzdandy
Copy link
Collaborator

xzdandy commented Nov 3, 2023

Hi @hershd23, you can define a function with error in the test file. And try to import that function. For my education, when is ModuleNotFound error? Is that only thrown out when the package is missing? How about the case that there are errors or typos in function definitions?

makes more sense. Will add tests for both the cases
	modified:   evadb/utils/generic_utils.py
	modified:   evadb/utils/generic_utils.py
	modified:   test/integration_tests/short/test_generic_utils.py
@hershd23
Copy link
Contributor Author

hershd23 commented Nov 4, 2023

@pchunduri6 @jarulraj this is up for a review, docs is failing for some reason have reported the issue. Other than that good enough to review

@jarulraj jarulraj merged commit 35b772b into georgia-tech-db:staging Nov 6, 2023
@jarulraj
Copy link
Member

jarulraj commented Nov 6, 2023

@hershd23 Can we add test cases for a file not found error?

@@ -50,17 +51,27 @@ def test_should_return_correct_class_for_path_without_classname(self):
assert vl.__qualname__ == DecordReader.__qualname__

def test_should_raise_on_missing_file(self):
with self.assertRaises(RuntimeError):
# Asserting on the error message, but that's brittle
with self.assertRaises(FileNotFoundError):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FileNotFoundError test case is already there prof @jarulraj

@hershd23
Copy link
Contributor Author

hershd23 commented Nov 6, 2023

Already added prof @jarulraj

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

More specific error messages for function loading
4 participants