jerryshao commented on PR #10256: URL: https://github.com/apache/gravitino/pull/10256#issuecomment-4096325929
After reviewing the code, I don't think the real code path will hit this issue. All 3 callers of `fromModelVersionPO` in `ModelVersionMetaService.java` are already protected: 1. **Line 106**: Called inside `Collectors.groupingBy(...)` — by definition, each group `m` always has ≥1 element. It is impossible for `m` to be empty here. 2. **Line 155** (`getModelVersionByIdentifier`): There is an explicit `if (modelVersionPOs.isEmpty())` check at line 135 that throws `NoSuchEntityException` before ever reaching `fromModelVersionPO`. 3. **Line 329** (`updateModelVersion`): Same pattern — explicit `if (oldModelVersionPOs.isEmpty())` check at line 308 throws `NoSuchEntityException` before reaching `fromModelVersionPO`. The only way to trigger the `IndexOutOfBoundsException` would be to call `fromModelVersionPO` directly with an empty list, which nothing in the codebase does. The `Preconditions` check added by this PR would effectively be dead code — it will never fire in practice. Given this, the value of this fix is questionable. It adds noise without addressing a real failure path. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
