lukecwik edited a comment on pull request #13069: URL: https://github.com/apache/beam/pull/13069#issuecomment-707911475
> On empty key vs. marker, sorry for going back and forth--let's settle this before you change the code again. > > Taking a step back, the question we want to answer is "do all ShardedKeys have a shard id" or is the shard id optional. If the former, I think representing it as a `(bytes, K)` tuple makes sense, but if we ever want to talk about "not having" a shard id than it should, logically, be `(Optional[bytes], K)` and we should use an explicit bit rather than have a "special" value of bytes that means "not there". Since we expect ShardedKey to be produced by the runner we don't expect the SDK/user to care so making it required simplifies user & SDK code. The runner is the only one that may care to differentiate unsharded vs sharded and can use any representation it wants to do so (e.g. by encoding Optional[bytes] into the bytes blob). The runners type doesn't have to match the user/SDK type. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected]
