Croway commented on PR #3089:
URL: https://github.com/apache/camel-kamelets/pull/3089#issuecomment-6020477773
I checked the 524 touched YAML files: all of them parse, have no duplicate
keys, keep everything outside `spec.template` unchanged, keep the same
`{{placeholders}}`, and add no new yamllint issues. The `library/` copies are
identical to `kamelets/`.
One problem: **the comments inside `spec.template` are dropped**, although
the description says comments are kept. It's 20 comment blocks in 16 Kamelets,
and most of them document security decisions (why a header family is stripped,
why identity is captured before a parse, why Reply-To stays disabled):
| Kamelet | Comment before |
|---|---|
| `aws-bedrock-agent-sink` | `removeHeaders` (CamelAwsBedrockAgent*
stripping) |
| `aws-ec2-sink` | `removeHeaders` (CamelAwsEC2* stripping) |
| `ceph-event-based-source` | `setHeader` (Ceph extension fields), `choice`
(Records envelope) |
| `counter-source` | `choice` (route scoped variable instead of a bean,
#2771) |
| `djl-image-to-text-action` | `bean` (WordNet hypernym) |
| `docling-convert-action` | `setProperty` (identity captured before the
conversion) |
| `kafka-source`, `kafka-apicurio-registry-not-secured-source`,
`kafka-azure-schema-registry-source`,
`kafka-not-secured-apicurio-registry-source`,
`kafka-not-secured-apicurio-registry-json-source` | `setHeader` (Kafka record
metadata) |
| `langchain4j-ingest-file-source` | `idempotentKey` |
| `langchain4j-ingest-sink` | `to` (the endpoint resolves the id) |
| `mail-sink` | `removeHeader` (drop upstream mail headers),
`useHeaderSubject` (why the useHeader* options are enabled) |
| `sap-netweaver-sink` | `setHeader` (CamelNetWeaverCommand pinned) |
| `tika-extract-text-action` | 4 blocks: identity capture, the `tika:`
endpoint, charset header sweep (the "keep in step with TikaTextDecode" note),
metadata header sweep |
Could you put them back in these 16 files, above the same steps, and
regenerate the `library/` copies?
### Can `camel validate normalize` keep them?
In apache/camel#27449, `KameletNormalizer` replaces `spec.template` with the
YAML from the route template dump. The dump is written from the route model,
and the model has no comments (the YAML parser drops them), so they can't
survive that path. They can be put back afterwards, though, and I think it's
worth doing:
- **Re-attach by position (worth it).** 17 of the 20 blocks sit right before
a step (`- removeHeaders:`, `- setHeader:`, ...). Normalization keeps the step
order and the EIP names, so each comment can be anchored to its step (its index
in the `steps` list and the EIP name). The other 3 sit before an option key
(`useHeaderSubject`, `idempotentKey`), and those keys are kept in
`parameters:`, so they can be anchored the same way. The one structural change
is the `split` whose following steps get nested inside it. There the anchor
can't always be found, so the comment should not be dropped silently.
- **Minimum (cheap, and needed anyway).** When `spec.template` has comments
the normalizer could not place back, print a warning that lists them, or fail
unless a flag is given. Losing a comment silently is what hurt here: the PR
checks compared the route-template dumps, and those can't show a lost comment.
- **Not recommended: convert comments to `note:`.**
`OptionalIdentifiedDefinition` has a `note` field, so the comments would
survive a model round-trip. But that changes the Kamelets' content (notes show
up in dumps and tooling), and it isn't a comment any more.
My suggestion: restore the comments by hand in this PR so it doesn't wait on
Camel. In apache/camel#27449 (or a follow-up), at least add the warning, and
ideally re-attach comments by step position.
--
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]
