Add config-specific exception hierarchy, validation checks, and CLI validation checks, and CLI error handling by idoudali · Pull Request #2319 · oumi-ai/oumi

idoudali · 2026-03-26T12:06:02Z

Description

Introduce a new lightweight oumi.exceptions module with an
OumiConfigError base class (kept import-cheap for the CLI).
Move HardwareException to oumi.exceptions. It remains importable
from both oumi.core.types and oumi.core.types.exceptions; the
latter is now a thin back-compat shim that re-exports from the
canonical location.
Wrap file I/O in BaseConfig.from_yaml, the save path, and
_read_config_without_interpolation to raise OumiConfigError on
FileNotFoundError, IsADirectoryError, and NotADirectoryError
instead of leaking raw OS errors.
Wrap adapter config loading in ModelParams to raise
OumiConfigError on OSError and json.JSONDecodeError, with
file path and parse location in the message.
Handle OumiConfigError in the CLI top-level handler so users see a
concise red error line and exit code 1 instead of a full traceback.
Update tests accordingly.

Related issues

Fixes # (issue)

Before submitting

This PR only changes documentation. (You can ignore the following checks in that case)
Did you read the contributor guideline Pull Request guidelines?
Did you link the issue(s) related to this PR in the section above?
Did you add / update tests where needed?

Reviewers

At least one review from a member of oumi-ai/oumi-staff is required.

Copilot

Pull request overview

Adds a configuration-specific exception hierarchy and uses it to fail fast on invalid/missing config file paths, then surfaces these failures as cleaner CLI errors.

Changes:

Introduce OumiConfigError / ConfigFileNotFoundError and export them from oumi.core.configs.
Add explicit “path exists and is a file” checks in config loading/saving and in LoRA adapter config discovery/reading (including wrapping read/JSON errors).
Add unit tests covering the new exception hierarchy and the new validation/error-handling paths.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/unit/core/configs/test_base_config.py	Adds tests for config exception hierarchy and file/path validation behaviors in `BaseConfig`.
tests/unit/core/configs/params/test_model_params.py	Adds tests for adapter config path/file validation and read/JSON error wrapping.
src/oumi/core/configs/params/model_params.py	Validates adapter config path is a file; wraps adapter-config read/JSON errors as `OumiConfigError`.
src/oumi/core/configs/exceptions.py	Introduces new config exception types.
src/oumi/core/configs/base_config.py	Adds file existence checks and wraps YAML save `OSError` as `OumiConfigError`.
src/oumi/core/configs/init.py	Re-exports the new config exception types.
src/oumi/cli/main.py	Adds a top-level `OumiConfigError` handler to print a user-friendly error and exit.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

oelachqar · 2026-04-14T21:52:07Z

+from oumi.exceptions import (
+    HardwareException,
+    OumiConfigError,
+    OumiConfigFileNotFoundError,


To avoid having too many exceptions we can probably remove OumiConfigFileNotFoundError and just use OumiConfigError

Removed the separate exception and have tried to simplify code

oelachqar · 2026-04-14T21:54:06Z

+            OmegaConf.save(config=processed_config, f=config_path)
+        except OSError as e:
+            # handle missing parent folder
+            raise OumiConfigError(f"Failed to save config to {config_path}: {e}") from e


I don't think this is a config error -- saving could fail for many reasons. We can check if the parent forlder more explicitely and raise the error then

Added a directory check and raise hopefully a better error message

oelachqar · 2026-04-14T21:54:44Z

+    """Base class for all configuration-related errors."""
+
+
+class OumiConfigFileNotFoundError(OumiConfigError, FileNotFoundError):


As discussed earlier let's remove this one

Removed the separate exception

gitar-bot · 2026-04-14T21:55:01Z

Gitar is working

_Gitar

idoudali · 2026-04-16T23:07:16Z

Sign in to view

+            if not Path(config_path).is_file():
+                raise OumiConfigError(
+                    f"Config file not found or path is not a file: {config_path}"
+                )


@oelachqar what do you think of our above opinion?

AFAIU, you prefer that a permission issue does fully manifest. A configuration mistake is e.g. to point to a folder that does not exist, but any permission issue does not fall under OumiConfigError.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Introduce a new lightweight `oumi.exceptions` module with an `OumiConfigError` base class (kept import-cheap for the CLI). - Move `HardwareException` to `oumi.exceptions`. It remains importable from both `oumi.core.types` and `oumi.core.types.exceptions`; the latter is now a thin back-compat shim that re-exports from the canonical location. - Wrap file I/O in `BaseConfig.from_yaml`, the save path, and `_read_config_without_interpolation` to raise `OumiConfigError` on `FileNotFoundError`, `IsADirectoryError`, and `NotADirectoryError` instead of leaking raw OS errors. - Wrap adapter config loading in `ModelParams` to raise `OumiConfigError` on `OSError` and `json.JSONDecodeError`, with file path and parse location in the message. - Handle `OumiConfigError` in the CLI top-level handler so users see a concise red error line and exit code 1 instead of a full traceback. - Update tests accordingly. Fixes OPE-1848

idoudali requested a review from oelachqar March 26, 2026 12:06

idoudali self-assigned this Mar 26, 2026

Copilot AI review requested due to automatic review settings March 26, 2026 12:06

Copilot started reviewing on behalf of idoudali March 26, 2026 12:06 View session

Copilot AI reviewed Mar 26, 2026

View reviewed changes

Comment thread src/oumi/core/configs/base_config.py Outdated

Comment thread src/oumi/cli/main.py Outdated

Comment thread src/oumi/cli/main.py

idoudali mentioned this pull request Mar 26, 2026

Add dataset_path existence check in DatasetParams finalize_and_validate #2320

Open

4 tasks

idoudali force-pushed the idoudali/config-exceptions branch from 14e626c to 35745d5 Compare March 26, 2026 16:19

idoudali requested a review from Copilot March 27, 2026 11:24

Copilot started reviewing on behalf of idoudali March 27, 2026 11:25 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

Comment thread src/oumi/core/configs/base_config.py Outdated

Comment thread src/oumi/cli/main.py Outdated

Comment thread src/oumi/core/configs/exceptions.py Outdated

idoudali force-pushed the idoudali/config-exceptions branch from 35745d5 to 9c851f3 Compare March 27, 2026 11:56

idoudali mentioned this pull request Mar 27, 2026

Validate explicitly-provided adapter_model path in ModelParams #2321

Closed

4 tasks

idoudali force-pushed the idoudali/config-exceptions branch from 9c851f3 to e5f549f Compare March 27, 2026 14:33

idoudali mentioned this pull request Mar 28, 2026

Replace config ValueError with OumiConfigValueError #2325

Open

4 tasks

oelachqar reviewed Apr 14, 2026

View reviewed changes

idoudali force-pushed the idoudali/config-exceptions branch from 11f1353 to a2e3d37 Compare April 16, 2026 22:10

sentry bot reviewed Apr 16, 2026

View reviewed changes

idoudali requested review from Copilot and oelachqar April 16, 2026 23:07

Copilot started reviewing on behalf of idoudali April 16, 2026 23:07 View session

Copilot AI reviewed Apr 16, 2026

View reviewed changes

Comment thread src/oumi/core/configs/base_config.py Outdated

Comment thread src/oumi/core/configs/base_config.py Outdated

Comment thread src/oumi/core/configs/params/model_params.py Outdated

idoudali force-pushed the idoudali/config-exceptions branch from 8165f19 to e46d02e Compare April 17, 2026 17:51

idoudali requested a review from Copilot April 17, 2026 17:51

Copilot started reviewing on behalf of idoudali April 17, 2026 17:52 View session

Copilot AI reviewed Apr 17, 2026

View reviewed changes

idoudali requested a review from Copilot April 17, 2026 18:02

Copilot started reviewing on behalf of idoudali April 17, 2026 18:03 View session

Copilot AI reviewed Apr 17, 2026

View reviewed changes

Comment thread src/oumi/core/types/__init__.py

idoudali force-pushed the idoudali/config-exceptions branch from e46d02e to 4804801 Compare April 17, 2026 20:14

oelachqar approved these changes Apr 17, 2026

View reviewed changes

		"""Base class for all configuration-related errors."""


		class OumiConfigFileNotFoundError(OumiConfigError, FileNotFoundError):

Conversation

idoudali commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues

Before submitting

Reviewers

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

oelachqar Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

idoudali Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

oelachqar Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

idoudali Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

oelachqar Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

idoudali Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

gitar-bot bot commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

idoudali Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

idoudali commented Mar 26, 2026 •

edited

Loading

gitar-bot bot commented Apr 14, 2026 •

edited

Loading