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

[CT-1567] Clean up exceptions.py #6339

Closed
3 tasks
emmyoop opened this issue Nov 29, 2022 · 3 comments · Fixed by #6347
Closed
3 tasks

[CT-1567] Clean up exceptions.py #6339

emmyoop opened this issue Nov 29, 2022 · 3 comments · Fixed by #6347
Assignees
Milestone

Comments

@emmyoop
Copy link
Member

emmyoop commented Nov 29, 2022

a precursor to #5958

There are a few goals here

  1. Stop using exception functions and just use exception classes
  2. Add a category to the exceptions that can be logged
  3. organize exceptions better in the file to be grouped by where they are triggered - ideally these could be split out into the directory they are raised but out of scope for now
  4. When the exception gets raised, pass in the parameters needed to build the message when possible
  5. Make sure these changes get trickled into adapters

Questions

  • Some exceptions build custom messages when raised. Should those be custom exceptions or continue to work as is.
  • Is an exception category its own thing or is it just another category at the top level of logs
  • The jinja context exceptions needs to persists. Could they live elsewhere? Should they?
@emmyoop emmyoop self-assigned this Nov 29, 2022
@github-actions github-actions bot changed the title Clean up exceptions.py [CT-1567] Clean up exceptions.py Nov 29, 2022
@VersusFacit
Copy link
Contributor

I would surmise from 1 that part of the work is get rid of bare adhoc exceptions too, right? Does this statement imply you are more keen on passing classes around that encode messages, like with error_or_warn? If so, you sure have my ax vote.

@emmyoop
Copy link
Member Author

emmyoop commented Nov 30, 2022

bare adhoc exceptions

@VersusFacit are you referring to things like RecursionException?

I'm proposing we move more toward a model like IncompatibleSchemaException where the message is built within the exception class instead of being built and then passed in. I don't know yet if this is possible for every single exception. That's part of the purpose of this task.

@emmyoop emmyoop mentioned this issue Dec 1, 2022
6 tasks
@VersusFacit
Copy link
Contributor

We're on same page! I was referring to raises on bare strings. I get some may not be able to converted, but I'm glad it's a priority to standardize what we can.

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 a pull request may close this issue.

3 participants