kennknowles commented on PR #30545: URL: https://github.com/apache/beam/pull/30545#issuecomment-2082643119
Yea, I actually do feel strongly about the one remaining issue. I get what you are saying about directly re-using the reshuffle translator as a way of expressing "the reshuffle translation is good enough for this use case", and this is a point where I diverge with many programmers. One helpful way I think about from the perspective of both Reshuffle and Redistribute translations being perpetual works-in-progress. If someone goes in and messes with the function "reshuffleTranslator" in either Flink or Dataflow, I don't expect them to be aware of all the places it is used. They should edit with confidence that the blast radius is just Reshuffle. So having separate-but-coincidentally-currently-identical pieces of code achieves that. Contrast that with the case where we intend one to always evolve with the other, due to logical necessity. In that case, we would also want them to only be thinking about Reshuffle, but anything implemented via Reshuffle would get the changes, because we want them to. So in this particular case, I want them to be as independent as possible. In fact, I want to actually re-deprecate Reshuffle as soon as we get RequiresStableInput implemented fully ;-) and I also want to drive all at-least-once usage to Redistribute. -- 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]
