loustler commented on issue #11655: URL: https://github.com/apache/seatunnel/issues/11655#issuecomment-5204881002
@SEZ9 thank you for the analysis — the framing in your comment is what this fix follows. It is up now as #11673. **On option (a) vs (b).** Went with (a), for exactly the reason you gave: dropping the relocation from the uber jar would fix this one call site but change the contract for every other consumer of that jar. Option (a) keeps the blast radius to a single module. One detail worth flagging, because it is easy to get wrong: the relocation in `seatunnel-hadoop-aws` has to use the **uber jar's** shaded pattern rather than a module-specific one. The point is to agree with the class that supplies the method — relocating to its own prefix would leave the descriptor just as mismatched, only differently. Nothing is added to the jar either; `maven-shade-plugin` rewrites references even when it bundles none of the target classes, so the class count is unchanged (194 under `org/apache/hadoop/` before and after) and only constant-pool entries differ. **On the regression test.** You asked for something that keeps this from *silently* reoccurring after future Hadoop upgrades. I went with a jar-level check rather than the unit test or the localstack case you suggested, and I would rather explain that than quietly substitute it. The check (`tools/dependencies/check_shaded_jackson_refs.py`, wired into the existing `dependency-license` job, which already builds the jar) scans every class in the shaded artifact for surviving references to the original package. It runs in seconds, with no container and no AWS credentials. The concrete argument for it is that it already found more than an assumed-role test would have. On hadoop-aws 3.4.3 it flags three classes beyond `RoleModel` — `SuccessData`, `PendingSet` and `SinglePendingCommit` — whose references are `@JsonProperty` **annotations**. Those fail differently from the `NoSuchMethodError`: the relocated Jackson looks for relocated annotations, does not find them, and does not throw. It falls back to default property naming and inclusion, so the S3A committer writes its `_SUCCESS` and `.pending` files with the wrong shape and nothing surfaces an error. That is precisely the silent recurrence you were pointing at, and no test exercising the assumed-role path would have reached it. That said, the two are not mutually exclusive. If you would still like a test that executes `RoleModel`/`JsonSerialization` against the shaded artifact, I am glad to add one — it just would not have caught the annotation case on its own, which is why the artifact-level check came first. Would appreciate your review on #11673 whenever you have time. -- 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]
