weiqingy opened a new pull request, #951: URL: https://github.com/apache/flink-agents/pull/951
Linked issue: none (hotfix) ### Purpose of change Two related cleanups in the skill module. **Repositories leaked on unexpected load failures.** `SkillManager._load_skills` released already-opened repositories only for `OSError` and `ValueError`. Any other failure skipped the cleanup, so the repositories' temp directories and atexit handlers leaked until interpreter exit. The caller never receives a `SkillManager` reference on a failed construction, so it cannot clean them up itself. Two concrete cases: a corrupt archive raises `zipfile.BadZipFile`, which does not inherit from `OSError`; and a failure inside `_register_repo` was outside the guarded region entirely. The source loop is now wrapped so the cleanup runs on every failure path. Which exceptions get wrapped in `RuntimeError` is unchanged: `OSError` and `ValueError` still wrap with the same message and `from e` chaining, and everything else still propagates unchanged in type and value. The only behavior change is that `close()` now runs. **Dead code removed.** `SkillManager.resolveResourcePath` and `SkillRepositoryInfo` have no callers. The resource-path method existed in both languages, so both are deleted in the same commit rather than leaving one side without its counterpart. Its only reference was a Java test, which is removed with it. ### Tests Two tests added to `TestSkillManagerMixedSources`, covering the two previously unguarded paths: a source raising `zipfile.BadZipFile` from `open()`, and a failure raised from `_register_repo` after an earlier source loaded cleanly. Both assert the earlier repository was closed and that the original exception propagates unwrapped. Both were verified to be discriminating. Re-narrowing the handler so it no longer spans `_register_repo` fails only the registration test while both pre-existing cleanup tests stay green, which is what makes them cover distinct regions rather than restating each other. Java `SkillManagerTest` 17 passed and the full `runtime` module 400 passed. Python skill module 68 passed and the full suite 619 passed, 11 skipped. `spotless:check`, `ruff check`, and `ruff format` are all clean. ### API No public API change. `SkillManager` is a runtime-internal component, and both removed symbols had no callers in either language. ### Documentation - [ ] `doc-needed` - [x] `doc-not-needed` - [ ] `doc-included` -- 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]
