fab icon indicating copy to clipboard operation
fab copied to clipboard

Replace RuntimeErrors with custom exceptions

Open t00sa opened this issue 4 months ago • 4 comments

Swap all RuntimeError exceptions for custom fab exceptions using a class hierarchy derived from RuntimeError to minimise disruptive changes.

These changes fall into three broad areas:

  • creation of a new set of error classes in fab.errors and an associated set of tests
  • changes to the existing regression tests to:
    • check the new message formats
    • check the exceptions - which are still caught as RuntimeError to test the impact on calling functions - match the expected instance of the new error class
  • replacement of raise RuntimeError across the code base with the new error classes

While the change is complete and the test suite runs successfully in my environment, I still have a couple of outstanding questions that need to be addressed in review:

  • are the names of the new exceptions classes acceptable? The top level class names end with Error but the derived classes do not. Ending every class name with Error would make fab more consistent with python, at the cost of longer exception names
  • are there any potential clashes or issues with the in-flight baf PR?

t00sa avatar Aug 22 '25 14:08 t00sa

I'm about to start a review but I'll leave this comment here for immediate consideration. Are you aware of FabException in https://github.com/MetOffice/fab/blob/main/source/fab/init.py? Have you considered how this interacts with your change?

MatthewHambley avatar Aug 26 '25 09:08 MatthewHambley

@yaswant , I am a bit confused: I already did a review, and as far as I can see no work was done on the PR (and neither was there a reply to my comments). Additionally, the branch has conflicts ;)

hiker avatar Oct 10 '25 14:10 hiker

@hiker please ignore my assignment. You are right, Sam has not responded to your comments yet. Apologies for the confusion.

yaswant avatar Oct 10 '25 15:10 yaswant

I have addressed the comments from the previous reviews, the CI tests pass, and I've successfully used my branch to build skeleton in LFRic core using Jason's branch, so I think this ready for another round of reviewing.

t00sa avatar Oct 23 '25 10:10 t00sa