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

Introduce MissingRequiredFilesToDeleteException for Streaming Deletes #11887

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

shanielh
Copy link
Contributor

Added a new exception, MissingRequiredFilesToDeleteException, to allow users
to inspect missing file details during streaming deletes.

  • This exception extends ValidationException and preserves compatibility
    with the existing validation message.
  • Users can now retrieve missing file locations via the getMissingLocations()
    method, instead of relying solely on the exception message.

Added a new exception, `MissingRequiredFilesToDeleteException`, to allow users
to inspect missing file details during streaming deletes.

- This exception extends `ValidationException` and preserves compatibility
  with the existing validation message.
- Users can now retrieve missing file locations via the `getMissingLocations()`
  method, instead of relying solely on the exception message.
@RussellSpitzer
Copy link
Member

I think generally we wouldn't want to introduce new API concepts unless there is some usage of that API within the core library itself (Otherwise we are basically just opening up technical debt without tests or benefit). Do we have plans for introducing a usage in the repo somewhere?

At minimum I think we would want tests utilizing this API but I think a practical application would also be required as a justification.

@shanielh
Copy link
Contributor Author

shanielh commented Jan 3, 2025

I think generally we wouldn't want to introduce new API concepts unless there is some usage of that API within the core library itself (Otherwise we are basically just opening up technical debt without tests or benefit). Do we have plans for introducing a usage in the repo somewhere?

At minimum I think we would want tests utilizing this API but I think a practical application would also be required as a justification.

I don't think there's a utilisation to the API in the core, my practical use is to check whether all files are already deleted or not in a Delete operation. This information already lies in the ValidationException, but it requires me to do string manipulation in order to get the information.

As a general rule of thumb, I suggest all information that lies in the exception message should be somehow accessible via API.

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

Successfully merging this pull request may close these issues.

2 participants