Fail later when duplicate transform translators are on the classpath #30252
Conversation
kennknowles
force-pushed
the
fail-cleaner
branch
from
b2a46e1
to
20b3c99
Compare
| @Override | ||
| public Map<String, ExpansionService.TransformProvider> knownTransforms() { | ||
| ImmutableMap.Builder<String, ExpansionService.TransformProvider> builder = |
There was a problem hiding this comment.
I was getting duplicate key errors from this builder, which is not built until after everything. So this is actually the thing that specifically I needed to refactor to get more messages in the middle. (and I did figure out it was a splittable keyed pardo registration or something like that)
There was a problem hiding this comment.
Ack. Didn't go through all this logic. Assume this is just a refactoring.
|
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
kennknowles
force-pushed
the
fail-cleaner
branch
2 times, most recently
from
8dcb92f
to
69940bd
Compare
This makes us get IllegalArgumentException and clearer stack traces than the NoClassDefFound error which will only show up in the first attempt.
kennknowles
force-pushed
the
fail-cleaner
branch
from
69940bd
to
9b4b509
Compare
| @Override | ||
| public Map<String, ExpansionService.TransformProvider> knownTransforms() { | ||
| ImmutableMap.Builder<String, ExpansionService.TransformProvider> builder = |
There was a problem hiding this comment.
Ack. Didn't go through all this logic. Assume this is just a refactoring.
| @SuppressWarnings({ | ||
| "rawtypes" // TODO(https://github.com/apache/beam/issues/20447) | ||
| }) | ||
| public interface TransformProvider<InputT extends PInput, OutputT extends POutput> { |
There was a problem hiding this comment.
Also, assuming this is just moving this from a inner class to a separate file. Lemme know if you want me to review this in detail.
There was a problem hiding this comment.
Yea it is just that. And I created the subclasses instead of anonymous subclasses to add the equals methods and all that. But just a boilerplate refactor.
This makes us get IllegalArgumentException and clearer stack traces than the NoClassDefFound error which will only show up in the first attempt.
Separating this change and related refactors from #30235
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.