gnodet-bot commented on code in PR #29774:
URL: https://github.com/apache/camel/pull/29774#discussion_r4556786234
##########
dsl/camel-kamelet-main/src/main/java/org/apache/camel/main/KameletMain.java:
##########
@@ -695,10 +695,15 @@ protected CamelContext createCamelContext() {
String sourceDir =
getInitialProperties().getProperty(getInstanceType() + ".sourceDir");
try {
+ // the dependencies the user gives (CLI, //DEPS, Kamelets): a run
needs them, so it fails when one cannot be
+ // downloaded, while exporting and transforming keep going as far
as they can
+ boolean requiredDependencies = !export && !transform;
Review Comment:
⚠️ **Design gap — Kamelet dependency failures are silently swallowed**
`requiredDependencies=true` is passed to `DependencyDownloaderStrategy`, so
when a Kamelet dependency cannot be downloaded, `onDependency()` will throw
`DownloadException`. However, the **caller** of `onDependency()` in
`KameletRoutesBuilderLoader.preConfigureDependencies()` (camel-yaml-dsl) wraps
every invocation in `catch (Exception e) { // ignore }`, which silently
swallows the exception:
```java
// KameletRoutesBuilderLoader.java:165 (not modified by this PR)
ds.onDependency(d);
} catch (Exception e) {
// ignore
}
```
This means `camel run` with a Kamelet that declares a missing dependency
will still start successfully (and fail at runtime with an obscure
class-loading error), contradicting the PR's stated goal of stopping on Kamelet
dependency failures.
Either:
1. Add a rethrow to `KameletRoutesBuilderLoader.preConfigureDependencies()`:
`catch (DownloadException e) { throw e; } catch (Exception e) { // ignore }`
2. Or scope the PR description to exclude Kamelet dependencies (the CLI and
`//DEPS` paths work correctly).
##########
dsl/camel-kamelet-main/src/main/java/org/apache/camel/main/download/DownloadThreadPool.java:
##########
@@ -78,9 +94,17 @@ void awaitCompletion(Logger log, Future<?> future, String
gav) {
// not done
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
+ future.cancel(true);
Review Comment:
ℹ️ **`future.cancel(true)` after `InterruptedException` from `future.get()`**
The interruption is on the *calling* thread (the thread waiting in
`future.get()`), not necessarily the download thread. `cancel(true)` sends an
interrupt to the download task — if still running, the task thread gets
interrupted mid-transfer. Maven Aether uses atomic file rename semantics, so
repository integrity is preserved, but the in-flight HTTP transfer will abort
and must be restarted on the next run.
This is the correct behavior (you want to stop the download when the caller
is interrupted), but worth documenting: a cancelled in-progress download leaves
no partial artifacts, it simply needs to be re-downloaded.
--
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]
